diff mbox series

[2/2] spapr/xive: Set the OS CAM line at reset

Message ID 20191017144241.12522-3-clg@kaod.org
State New
Headers show
Series spapr: interrupt presenter fixes | expand

Commit Message

Cédric Le Goater Oct. 17, 2019, 2:42 p.m. UTC
When a Virtual Processor is scheduled to run on a HW thread, the
hypervisor pushes its identifier in the OS CAM line. When running in
TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.

Introduce a 'os-cam' property which will be used to set the OS CAM
line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
are done when the XIVE interrupt controller are activated.

This change also has the benefit to remove the use of CPU_FOREACH()
which can be unsafe.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/spapr_xive.h |  1 -
 include/hw/ppc/xive.h       |  4 +++-
 hw/intc/spapr_xive.c        | 31 +++++--------------------------
 hw/intc/xive.c              | 22 +++++++++++++++++++++-
 hw/ppc/pnv.c                |  3 ++-
 5 files changed, 31 insertions(+), 30 deletions(-)

Comments

David Gibson Oct. 18, 2019, 3:55 a.m. UTC | #1
On Thu, Oct 17, 2019 at 04:42:41PM +0200, Cédric Le Goater wrote:
> When a Virtual Processor is scheduled to run on a HW thread, the
> hypervisor pushes its identifier in the OS CAM line. When running in
> TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.
> 
> Introduce a 'os-cam' property which will be used to set the OS CAM
> line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
> are done when the XIVE interrupt controller are activated.

I'm not immediately seeing the advantage of doing this via a property,
rather than poking it from the PAPR code which already knows the right
values.

Also, let me check my understanding:
  IIUC, on powernv the OS (running in HV mode) can alter the OS CAM
  lines for itself and/or its guests, but for pseries they're fixed in
  place.  Is that right?

> This change also has the benefit to remove the use of CPU_FOREACH()
> which can be unsafe.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/spapr_xive.h |  1 -
>  include/hw/ppc/xive.h       |  4 +++-
>  hw/intc/spapr_xive.c        | 31 +++++--------------------------
>  hw/intc/xive.c              | 22 +++++++++++++++++++++-
>  hw/ppc/pnv.c                |  3 ++-
>  5 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index d84bd5c229f0..742b7e834f2a 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -57,7 +57,6 @@ typedef struct SpaprXive {
>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
>  
>  void spapr_xive_hcall_init(SpaprMachineState *spapr);
> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>  void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable);
>  void spapr_xive_map_mmio(SpaprXive *xive);
>  
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 99381639f50c..e273069c25a9 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -319,6 +319,7 @@ typedef struct XiveTCTX {
>      qemu_irq    os_output;
>  
>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +    uint32_t    os_cam;
>  } XiveTCTX;
>  
>  /*
> @@ -414,7 +415,8 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>  uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>  
>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
> +                         Error **errp);
>  void xive_tctx_reset(XiveTCTX *tctx);
>  
>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 0c3acf1a4192..71f138512a1c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -205,21 +205,13 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable)
>      memory_region_set_enabled(&xive->end_source.esb_mmio, false);
>  }
>  
> -/*
> - * When a Virtual Processor is scheduled to run on a HW thread, the
> - * hypervisor pushes its identifier in the OS CAM line. Emulate the
> - * same behavior under QEMU.
> - */
> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
> +static uint32_t spapr_xive_get_os_cam(PowerPCCPU *cpu)
>  {
>      uint8_t  nvt_blk;
>      uint32_t nvt_idx;
> -    uint32_t nvt_cam;
> -
> -    spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx);
>  
> -    nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx));
> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
> +    spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
> +    return xive_nvt_cam_line(nvt_blk, nvt_idx);
>  }
>  
>  static void spapr_xive_end_reset(XiveEND *end)
> @@ -537,19 +529,14 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
>      SpaprXive *xive = SPAPR_XIVE(intc);
>      Object *obj;
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +    uint32_t os_cam = spapr_xive_get_os_cam(cpu);
>  
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), os_cam, errp);
>      if (!obj) {
>          return -1;
>      }
>  
>      spapr_cpu->tctx = XIVE_TCTX(obj);
> -
> -    /*
> -     * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
> -     * don't beneficiate from the reset of the XIVE IRQ backend
> -     */
> -    spapr_xive_set_tctx_os_cam(spapr_cpu->tctx);
>      return 0;
>  }
>  
> @@ -650,14 +637,6 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
>  static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp)
>  {
>      SpaprXive *xive = SPAPR_XIVE(intc);
> -    CPUState *cs;
> -
> -    CPU_FOREACH(cs) {
> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
> -
> -        /* (TCG) Set the OS CAM line of the thread interrupt context. */
> -        spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
> -    }
>  
>      if (kvm_enabled()) {
>          int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 0ae3f9b1efe4..be4f2c974178 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -566,6 +566,18 @@ static void xive_tctx_reset_handler(void *dev)
>          ipb_to_pipr(tctx->regs[TM_QW1_OS + TM_IPB]);
>      tctx->regs[TM_QW3_HV_PHYS + TM_PIPR] =
>          ipb_to_pipr(tctx->regs[TM_QW3_HV_PHYS + TM_IPB]);
> +
> +    /*
> +     * (TCG) Set the OS CAM line of the thread interrupt context.
> +     *
> +     * When a Virtual Processor is scheduled to run on a HW thread,
> +     * the hypervisor pushes its identifier in the OS CAM line.
> +     * Emulate the same behavior under QEMU.
> +     */
> +    if (tctx->os_cam) {
> +        uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | tctx->os_cam);
> +        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
> +    }
>  }
>  
>  void xive_tctx_reset(XiveTCTX *tctx)
> @@ -667,11 +679,17 @@ static const VMStateDescription vmstate_xive_tctx = {
>      },
>  };
>  
> +static Property  xive_tctx_properties[] = {
> +    DEFINE_PROP_UINT32("os-cam", XiveTCTX, os_cam, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void xive_tctx_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->desc = "XIVE Interrupt Thread Context";
> +    dc->props = xive_tctx_properties;
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> @@ -689,7 +707,8 @@ static const TypeInfo xive_tctx_info = {
>      .class_init    = xive_tctx_class_init,
>  };
>  
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
> +                         Error **errp)
>  {
>      Error *local_err = NULL;
>      Object *obj;
> @@ -698,6 +717,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>      object_unref(obj);
>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> +    object_property_set_int(obj, os_cam, "os-cam", &local_err);
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
>          goto error;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7cf64b6d2533..99c06842573e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -806,7 +806,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>       * controller object is initialized afterwards. Hopefully, it's
>       * only used at runtime.
>       */
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 0,
> +                           &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
Greg Kurz Oct. 18, 2019, 8:07 a.m. UTC | #2
On Thu, 17 Oct 2019 16:42:41 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> When a Virtual Processor is scheduled to run on a HW thread, the
> hypervisor pushes its identifier in the OS CAM line. When running in
> TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.
> 

This is only related to kernel_irqchip=off, which is always the case
when running in TCG actually. Maybe rephrase to "When not running with
an in-kernel irqchip, QEMU needs..." ?

> Introduce a 'os-cam' property which will be used to set the OS CAM
> line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
> are done when the XIVE interrupt controller are activated.
> 

Since OS CAM is constant, I guess it is ok to make it a property.
Alternatively, you could pass it as an extra parameter to
xive_tctx_reset().

> This change also has the benefit to remove the use of CPU_FOREACH()
> which can be unsafe.
> 

Nice !

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/spapr_xive.h |  1 -
>  include/hw/ppc/xive.h       |  4 +++-
>  hw/intc/spapr_xive.c        | 31 +++++--------------------------
>  hw/intc/xive.c              | 22 +++++++++++++++++++++-
>  hw/ppc/pnv.c                |  3 ++-
>  5 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index d84bd5c229f0..742b7e834f2a 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -57,7 +57,6 @@ typedef struct SpaprXive {
>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
>  
>  void spapr_xive_hcall_init(SpaprMachineState *spapr);
> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>  void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable);
>  void spapr_xive_map_mmio(SpaprXive *xive);
>  
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 99381639f50c..e273069c25a9 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -319,6 +319,7 @@ typedef struct XiveTCTX {
>      qemu_irq    os_output;
>  
>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +    uint32_t    os_cam;
>  } XiveTCTX;
>  
>  /*
> @@ -414,7 +415,8 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>  uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>  
>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
> +                         Error **errp);
>  void xive_tctx_reset(XiveTCTX *tctx);
>  
>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 0c3acf1a4192..71f138512a1c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -205,21 +205,13 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable)
>      memory_region_set_enabled(&xive->end_source.esb_mmio, false);
>  }
>  
> -/*
> - * When a Virtual Processor is scheduled to run on a HW thread, the
> - * hypervisor pushes its identifier in the OS CAM line. Emulate the
> - * same behavior under QEMU.
> - */
> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
> +static uint32_t spapr_xive_get_os_cam(PowerPCCPU *cpu)
>  {
>      uint8_t  nvt_blk;
>      uint32_t nvt_idx;
> -    uint32_t nvt_cam;
> -
> -    spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx);
>  
> -    nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx));
> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
> +    spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
> +    return xive_nvt_cam_line(nvt_blk, nvt_idx);
>  }
>  
>  static void spapr_xive_end_reset(XiveEND *end)
> @@ -537,19 +529,14 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
>      SpaprXive *xive = SPAPR_XIVE(intc);
>      Object *obj;
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +    uint32_t os_cam = spapr_xive_get_os_cam(cpu);
>  
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), os_cam, errp);
>      if (!obj) {
>          return -1;
>      }
>  
>      spapr_cpu->tctx = XIVE_TCTX(obj);
> -
> -    /*
> -     * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
> -     * don't beneficiate from the reset of the XIVE IRQ backend
> -     */
> -    spapr_xive_set_tctx_os_cam(spapr_cpu->tctx);
>      return 0;
>  }
>  
> @@ -650,14 +637,6 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
>  static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp)
>  {
>      SpaprXive *xive = SPAPR_XIVE(intc);
> -    CPUState *cs;
> -
> -    CPU_FOREACH(cs) {
> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
> -
> -        /* (TCG) Set the OS CAM line of the thread interrupt context. */
> -        spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
> -    }
>  
>      if (kvm_enabled()) {
>          int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 0ae3f9b1efe4..be4f2c974178 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -566,6 +566,18 @@ static void xive_tctx_reset_handler(void *dev)
>          ipb_to_pipr(tctx->regs[TM_QW1_OS + TM_IPB]);
>      tctx->regs[TM_QW3_HV_PHYS + TM_PIPR] =
>          ipb_to_pipr(tctx->regs[TM_QW3_HV_PHYS + TM_IPB]);
> +
> +    /*
> +     * (TCG) Set the OS CAM line of the thread interrupt context.

As per my remark above, this shouldn't mention TCG but rather
kernel-irqchip=off.

> +     *
> +     * When a Virtual Processor is scheduled to run on a HW thread,
> +     * the hypervisor pushes its identifier in the OS CAM line.
> +     * Emulate the same behavior under QEMU.
> +     */
> +    if (tctx->os_cam) {
> +        uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | tctx->os_cam);
> +        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
> +    }
>  }
>  
>  void xive_tctx_reset(XiveTCTX *tctx)
> @@ -667,11 +679,17 @@ static const VMStateDescription vmstate_xive_tctx = {
>      },
>  };
>  
> +static Property  xive_tctx_properties[] = {
> +    DEFINE_PROP_UINT32("os-cam", XiveTCTX, os_cam, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void xive_tctx_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->desc = "XIVE Interrupt Thread Context";
> +    dc->props = xive_tctx_properties;
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> @@ -689,7 +707,8 @@ static const TypeInfo xive_tctx_info = {
>      .class_init    = xive_tctx_class_init,
>  };
>  
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
> +                         Error **errp)
>  {
>      Error *local_err = NULL;
>      Object *obj;
> @@ -698,6 +717,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>      object_unref(obj);
>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> +    object_property_set_int(obj, os_cam, "os-cam", &local_err);
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
>          goto error;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7cf64b6d2533..99c06842573e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -806,7 +806,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>       * controller object is initialized afterwards. Hopefully, it's
>       * only used at runtime.
>       */
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 0,
> +                           &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
Cédric Le Goater Oct. 18, 2019, 8:29 a.m. UTC | #3
On 18/10/2019 10:07, Greg Kurz wrote:
> On Thu, 17 Oct 2019 16:42:41 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> When a Virtual Processor is scheduled to run on a HW thread, the
>> hypervisor pushes its identifier in the OS CAM line. When running in
>> TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.
>>
> 
> This is only related to kernel_irqchip=off, which is always the case
> when running in TCG actually. Maybe rephrase to "When not running with
> an in-kernel irqchip, QEMU needs..." ?

yes. 


>> Introduce a 'os-cam' property which will be used to set the OS CAM
>> line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
>> are done when the XIVE interrupt controller are activated.
>>
> 
> Since OS CAM is constant, I guess it is ok to make it a property.
> Alternatively, you could pass it as an extra parameter to
> xive_tctx_reset().


indeed. We have all we need to do that. I will wait for some feedback.

>> This change also has the benefit to remove the use of CPU_FOREACH()
>> which can be unsafe.
>>
> 
> Nice !
> 
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/spapr_xive.h |  1 -
>>  include/hw/ppc/xive.h       |  4 +++-
>>  hw/intc/spapr_xive.c        | 31 +++++--------------------------
>>  hw/intc/xive.c              | 22 +++++++++++++++++++++-
>>  hw/ppc/pnv.c                |  3 ++-
>>  5 files changed, 31 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index d84bd5c229f0..742b7e834f2a 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -57,7 +57,6 @@ typedef struct SpaprXive {
>>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
>>  
>>  void spapr_xive_hcall_init(SpaprMachineState *spapr);
>> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>>  void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable);
>>  void spapr_xive_map_mmio(SpaprXive *xive);
>>  
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 99381639f50c..e273069c25a9 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -319,6 +319,7 @@ typedef struct XiveTCTX {
>>      qemu_irq    os_output;
>>  
>>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
>> +    uint32_t    os_cam;
>>  } XiveTCTX;
>>  
>>  /*
>> @@ -414,7 +415,8 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>>  uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>>  
>>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
>> +                         Error **errp);
>>  void xive_tctx_reset(XiveTCTX *tctx);
>>  
>>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 0c3acf1a4192..71f138512a1c 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -205,21 +205,13 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable)
>>      memory_region_set_enabled(&xive->end_source.esb_mmio, false);
>>  }
>>  
>> -/*
>> - * When a Virtual Processor is scheduled to run on a HW thread, the
>> - * hypervisor pushes its identifier in the OS CAM line. Emulate the
>> - * same behavior under QEMU.
>> - */
>> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
>> +static uint32_t spapr_xive_get_os_cam(PowerPCCPU *cpu)
>>  {
>>      uint8_t  nvt_blk;
>>      uint32_t nvt_idx;
>> -    uint32_t nvt_cam;
>> -
>> -    spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx);
>>  
>> -    nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx));
>> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
>> +    spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
>> +    return xive_nvt_cam_line(nvt_blk, nvt_idx);
>>  }
>>  
>>  static void spapr_xive_end_reset(XiveEND *end)
>> @@ -537,19 +529,14 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
>>      SpaprXive *xive = SPAPR_XIVE(intc);
>>      Object *obj;
>>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>> +    uint32_t os_cam = spapr_xive_get_os_cam(cpu);
>>  
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
>> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), os_cam, errp);
>>      if (!obj) {
>>          return -1;
>>      }
>>  
>>      spapr_cpu->tctx = XIVE_TCTX(obj);
>> -
>> -    /*
>> -     * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
>> -     * don't beneficiate from the reset of the XIVE IRQ backend
>> -     */
>> -    spapr_xive_set_tctx_os_cam(spapr_cpu->tctx);
>>      return 0;
>>  }
>>  
>> @@ -650,14 +637,6 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
>>  static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp)
>>  {
>>      SpaprXive *xive = SPAPR_XIVE(intc);
>> -    CPUState *cs;
>> -
>> -    CPU_FOREACH(cs) {
>> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> -
>> -        /* (TCG) Set the OS CAM line of the thread interrupt context. */
>> -        spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
>> -    }
>>  
>>      if (kvm_enabled()) {
>>          int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 0ae3f9b1efe4..be4f2c974178 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -566,6 +566,18 @@ static void xive_tctx_reset_handler(void *dev)
>>          ipb_to_pipr(tctx->regs[TM_QW1_OS + TM_IPB]);
>>      tctx->regs[TM_QW3_HV_PHYS + TM_PIPR] =
>>          ipb_to_pipr(tctx->regs[TM_QW3_HV_PHYS + TM_IPB]);
>> +
>> +    /*
>> +     * (TCG) Set the OS CAM line of the thread interrupt context.
> 
> As per my remark above, this shouldn't mention TCG but rather
> kernel-irqchip=off.

ok.

Thanks,

C.

> 
>> +     *
>> +     * When a Virtual Processor is scheduled to run on a HW thread,
>> +     * the hypervisor pushes its identifier in the OS CAM line.
>> +     * Emulate the same behavior under QEMU.
>> +     */
>> +    if (tctx->os_cam) {
>> +        uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | tctx->os_cam);
>> +        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
>> +    }
>>  }
>>  
>>  void xive_tctx_reset(XiveTCTX *tctx)
>> @@ -667,11 +679,17 @@ static const VMStateDescription vmstate_xive_tctx = {
>>      },
>>  };
>>  
>> +static Property  xive_tctx_properties[] = {
>> +    DEFINE_PROP_UINT32("os-cam", XiveTCTX, os_cam, 0),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>  static void xive_tctx_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>  
>>      dc->desc = "XIVE Interrupt Thread Context";
>> +    dc->props = xive_tctx_properties;
>>      dc->realize = xive_tctx_realize;
>>      dc->unrealize = xive_tctx_unrealize;
>>      dc->vmsd = &vmstate_xive_tctx;
>> @@ -689,7 +707,8 @@ static const TypeInfo xive_tctx_info = {
>>      .class_init    = xive_tctx_class_init,
>>  };
>>  
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
>> +                         Error **errp)
>>  {
>>      Error *local_err = NULL;
>>      Object *obj;
>> @@ -698,6 +717,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>>      object_unref(obj);
>>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
>> +    object_property_set_int(obj, os_cam, "os-cam", &local_err);
>>      object_property_set_bool(obj, true, "realized", &local_err);
>>      if (local_err) {
>>          goto error;
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 7cf64b6d2533..99c06842573e 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -806,7 +806,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>>       * controller object is initialized afterwards. Hopefully, it's
>>       * only used at runtime.
>>       */
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
>> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 0,
>> +                           &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>>          return;
>
Cédric Le Goater Oct. 18, 2019, 9:42 a.m. UTC | #4
On 18/10/2019 05:55, David Gibson wrote:
> On Thu, Oct 17, 2019 at 04:42:41PM +0200, Cédric Le Goater wrote:
>> When a Virtual Processor is scheduled to run on a HW thread, the
>> hypervisor pushes its identifier in the OS CAM line. When running in
>> TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.
>>
>> Introduce a 'os-cam' property which will be used to set the OS CAM
>> line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
>> are done when the XIVE interrupt controller are activated.
> 
> I'm not immediately seeing the advantage of doing this via a property,
> rather than poking it from the PAPR code which already knows the right
> values.

we can simplify by passing the OS CAM line value as a parameter of the 
xive_tctx_reset routine, as suggested by Greg.

> 
> Also, let me check my understanding:
>   IIUC, on powernv the OS (running in HV mode) can alter the OS CAM
>   lines for itself 

OPAL only sets the VT bit in the HW cam line.

Linux PowerNV sets the POOL CAM line.

> and/or its guests, 

KVM sets the OS CAM line when a vCPU is scheduled to run.

> but for pseries they're fixed in place.  Is that right?

QEMU emulates KVM and sets the OS CAM line to a value similar to what KVM
would use. We can consider this value a reset constant.

C.

 
>> This change also has the benefit to remove the use of CPU_FOREACH()
>> which can be unsafe.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/spapr_xive.h |  1 -
>>  include/hw/ppc/xive.h       |  4 +++-
>>  hw/intc/spapr_xive.c        | 31 +++++--------------------------
>>  hw/intc/xive.c              | 22 +++++++++++++++++++++-
>>  hw/ppc/pnv.c                |  3 ++-
>>  5 files changed, 31 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index d84bd5c229f0..742b7e834f2a 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -57,7 +57,6 @@ typedef struct SpaprXive {
>>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
>>  
>>  void spapr_xive_hcall_init(SpaprMachineState *spapr);
>> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>>  void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable);
>>  void spapr_xive_map_mmio(SpaprXive *xive);
>>  
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 99381639f50c..e273069c25a9 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -319,6 +319,7 @@ typedef struct XiveTCTX {
>>      qemu_irq    os_output;
>>  
>>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
>> +    uint32_t    os_cam;
>>  } XiveTCTX;
>>  
>>  /*
>> @@ -414,7 +415,8 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>>  uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>>  
>>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
>> +                         Error **errp);
>>  void xive_tctx_reset(XiveTCTX *tctx);
>>  
>>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 0c3acf1a4192..71f138512a1c 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -205,21 +205,13 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable)
>>      memory_region_set_enabled(&xive->end_source.esb_mmio, false);
>>  }
>>  
>> -/*
>> - * When a Virtual Processor is scheduled to run on a HW thread, the
>> - * hypervisor pushes its identifier in the OS CAM line. Emulate the
>> - * same behavior under QEMU.
>> - */
>> -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
>> +static uint32_t spapr_xive_get_os_cam(PowerPCCPU *cpu)
>>  {
>>      uint8_t  nvt_blk;
>>      uint32_t nvt_idx;
>> -    uint32_t nvt_cam;
>> -
>> -    spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx);
>>  
>> -    nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx));
>> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
>> +    spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
>> +    return xive_nvt_cam_line(nvt_blk, nvt_idx);
>>  }
>>  
>>  static void spapr_xive_end_reset(XiveEND *end)
>> @@ -537,19 +529,14 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
>>      SpaprXive *xive = SPAPR_XIVE(intc);
>>      Object *obj;
>>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>> +    uint32_t os_cam = spapr_xive_get_os_cam(cpu);
>>  
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
>> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), os_cam, errp);
>>      if (!obj) {
>>          return -1;
>>      }
>>  
>>      spapr_cpu->tctx = XIVE_TCTX(obj);
>> -
>> -    /*
>> -     * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
>> -     * don't beneficiate from the reset of the XIVE IRQ backend
>> -     */
>> -    spapr_xive_set_tctx_os_cam(spapr_cpu->tctx);
>>      return 0;
>>  }
>>  
>> @@ -650,14 +637,6 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
>>  static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp)
>>  {
>>      SpaprXive *xive = SPAPR_XIVE(intc);
>> -    CPUState *cs;
>> -
>> -    CPU_FOREACH(cs) {
>> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> -
>> -        /* (TCG) Set the OS CAM line of the thread interrupt context. */
>> -        spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
>> -    }
>>  
>>      if (kvm_enabled()) {
>>          int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 0ae3f9b1efe4..be4f2c974178 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -566,6 +566,18 @@ static void xive_tctx_reset_handler(void *dev)
>>          ipb_to_pipr(tctx->regs[TM_QW1_OS + TM_IPB]);
>>      tctx->regs[TM_QW3_HV_PHYS + TM_PIPR] =
>>          ipb_to_pipr(tctx->regs[TM_QW3_HV_PHYS + TM_IPB]);
>> +
>> +    /*
>> +     * (TCG) Set the OS CAM line of the thread interrupt context.
>> +     *
>> +     * When a Virtual Processor is scheduled to run on a HW thread,
>> +     * the hypervisor pushes its identifier in the OS CAM line.
>> +     * Emulate the same behavior under QEMU.
>> +     */
>> +    if (tctx->os_cam) {
>> +        uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | tctx->os_cam);
>> +        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
>> +    }
>>  }
>>  
>>  void xive_tctx_reset(XiveTCTX *tctx)
>> @@ -667,11 +679,17 @@ static const VMStateDescription vmstate_xive_tctx = {
>>      },
>>  };
>>  
>> +static Property  xive_tctx_properties[] = {
>> +    DEFINE_PROP_UINT32("os-cam", XiveTCTX, os_cam, 0),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>  static void xive_tctx_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>  
>>      dc->desc = "XIVE Interrupt Thread Context";
>> +    dc->props = xive_tctx_properties;
>>      dc->realize = xive_tctx_realize;
>>      dc->unrealize = xive_tctx_unrealize;
>>      dc->vmsd = &vmstate_xive_tctx;
>> @@ -689,7 +707,8 @@ static const TypeInfo xive_tctx_info = {
>>      .class_init    = xive_tctx_class_init,
>>  };
>>  
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>> +Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
>> +                         Error **errp)
>>  {
>>      Error *local_err = NULL;
>>      Object *obj;
>> @@ -698,6 +717,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>>      object_unref(obj);
>>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
>> +    object_property_set_int(obj, os_cam, "os-cam", &local_err);
>>      object_property_set_bool(obj, true, "realized", &local_err);
>>      if (local_err) {
>>          goto error;
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 7cf64b6d2533..99c06842573e 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -806,7 +806,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>>       * controller object is initialized afterwards. Hopefully, it's
>>       * only used at runtime.
>>       */
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
>> +    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 0,
>> +                           &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>>          return;
>
Cédric Le Goater Oct. 18, 2019, 10:41 a.m. UTC | #5
On 18/10/2019 11:42, Cédric Le Goater wrote:
> On 18/10/2019 05:55, David Gibson wrote:
>> On Thu, Oct 17, 2019 at 04:42:41PM +0200, Cédric Le Goater wrote:
>>> When a Virtual Processor is scheduled to run on a HW thread, the
>>> hypervisor pushes its identifier in the OS CAM line. When running in
>>> TCG or kernel_irqchip=off, QEMU needs to emulate the same behavior.
>>>
>>> Introduce a 'os-cam' property which will be used to set the OS CAM
>>> line at reset and remove the spapr_xive_set_tctx_os_cam() calls which
>>> are done when the XIVE interrupt controller are activated.
>>
>> I'm not immediately seeing the advantage of doing this via a property,
>> rather than poking it from the PAPR code which already knows the right
>> values.
> 
> we can simplify by passing the OS CAM line value as a parameter of the 
> xive_tctx_reset routine, as suggested by Greg.

and if we remove the reset handlers from XiveTCTX and rely only on the 
CPU reset handler to reset the presenter. 

C.
diff mbox series

Patch

diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index d84bd5c229f0..742b7e834f2a 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -57,7 +57,6 @@  typedef struct SpaprXive {
 void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
 
 void spapr_xive_hcall_init(SpaprMachineState *spapr);
-void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
 void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable);
 void spapr_xive_map_mmio(SpaprXive *xive);
 
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 99381639f50c..e273069c25a9 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -319,6 +319,7 @@  typedef struct XiveTCTX {
     qemu_irq    os_output;
 
     uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
+    uint32_t    os_cam;
 } XiveTCTX;
 
 /*
@@ -414,7 +415,8 @@  void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
 uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
 
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
+Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
+                         Error **errp);
 void xive_tctx_reset(XiveTCTX *tctx);
 
 static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 0c3acf1a4192..71f138512a1c 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -205,21 +205,13 @@  void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable)
     memory_region_set_enabled(&xive->end_source.esb_mmio, false);
 }
 
-/*
- * When a Virtual Processor is scheduled to run on a HW thread, the
- * hypervisor pushes its identifier in the OS CAM line. Emulate the
- * same behavior under QEMU.
- */
-void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
+static uint32_t spapr_xive_get_os_cam(PowerPCCPU *cpu)
 {
     uint8_t  nvt_blk;
     uint32_t nvt_idx;
-    uint32_t nvt_cam;
-
-    spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx);
 
-    nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx));
-    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
+    spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
+    return xive_nvt_cam_line(nvt_blk, nvt_idx);
 }
 
 static void spapr_xive_end_reset(XiveEND *end)
@@ -537,19 +529,14 @@  static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc,
     SpaprXive *xive = SPAPR_XIVE(intc);
     Object *obj;
     SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+    uint32_t os_cam = spapr_xive_get_os_cam(cpu);
 
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp);
+    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), os_cam, errp);
     if (!obj) {
         return -1;
     }
 
     spapr_cpu->tctx = XIVE_TCTX(obj);
-
-    /*
-     * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
-     * don't beneficiate from the reset of the XIVE IRQ backend
-     */
-    spapr_xive_set_tctx_os_cam(spapr_cpu->tctx);
     return 0;
 }
 
@@ -650,14 +637,6 @@  static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers,
 static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp)
 {
     SpaprXive *xive = SPAPR_XIVE(intc);
-    CPUState *cs;
-
-    CPU_FOREACH(cs) {
-        PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-        /* (TCG) Set the OS CAM line of the thread interrupt context. */
-        spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
-    }
 
     if (kvm_enabled()) {
         int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 0ae3f9b1efe4..be4f2c974178 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -566,6 +566,18 @@  static void xive_tctx_reset_handler(void *dev)
         ipb_to_pipr(tctx->regs[TM_QW1_OS + TM_IPB]);
     tctx->regs[TM_QW3_HV_PHYS + TM_PIPR] =
         ipb_to_pipr(tctx->regs[TM_QW3_HV_PHYS + TM_IPB]);
+
+    /*
+     * (TCG) Set the OS CAM line of the thread interrupt context.
+     *
+     * When a Virtual Processor is scheduled to run on a HW thread,
+     * the hypervisor pushes its identifier in the OS CAM line.
+     * Emulate the same behavior under QEMU.
+     */
+    if (tctx->os_cam) {
+        uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | tctx->os_cam);
+        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+    }
 }
 
 void xive_tctx_reset(XiveTCTX *tctx)
@@ -667,11 +679,17 @@  static const VMStateDescription vmstate_xive_tctx = {
     },
 };
 
+static Property  xive_tctx_properties[] = {
+    DEFINE_PROP_UINT32("os-cam", XiveTCTX, os_cam, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void xive_tctx_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->desc = "XIVE Interrupt Thread Context";
+    dc->props = xive_tctx_properties;
     dc->realize = xive_tctx_realize;
     dc->unrealize = xive_tctx_unrealize;
     dc->vmsd = &vmstate_xive_tctx;
@@ -689,7 +707,8 @@  static const TypeInfo xive_tctx_info = {
     .class_init    = xive_tctx_class_init,
 };
 
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
+Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, uint32_t os_cam,
+                         Error **errp)
 {
     Error *local_err = NULL;
     Object *obj;
@@ -698,6 +717,7 @@  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
     object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
     object_unref(obj);
     object_property_add_const_link(obj, "cpu", cpu, &error_abort);
+    object_property_set_int(obj, os_cam, "os-cam", &local_err);
     object_property_set_bool(obj, true, "realized", &local_err);
     if (local_err) {
         goto error;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7cf64b6d2533..99c06842573e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -806,7 +806,8 @@  static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
      * controller object is initialized afterwards. Hopefully, it's
      * only used at runtime.
      */
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
+    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 0,
+                           &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;