[PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[Frederic Barrat]

When accessing a thread context through the IC BAR, the offset of the
page in the BAR identifies the CPU. From that offset, we can compute
the PIR (processor ID register) of the CPU to do the data structure
lookup. On P10, the current code assumes an access for node 0 when
computing the PIR. Everything is almost in place to allow access for
other nodes though. So this patch reworks how the PIR value is
computed so that we can access all thread contexts through the IC BAR.

The PIR is already correct on P9, so no need to modify anything there.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/intc/pnv_xive2.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index a39e070e82..f31c53c28d 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -1574,6 +1574,12 @@ static const MemoryRegionOps pnv_xive2_ic_sync_ops = {
  * When the TM direct pages of the IC controller are accessed, the
  * target HW thread is deduced from the page offset.
  */
+static uint32_t pnv_xive2_ic_tm_get_pir(PnvXive2 *xive, hwaddr offset)
+{
+    /* On P10, the node ID shift in the PIR register is 8 bits */
+    return xive->chip->chip_id << 8 | offset >> xive->ic_shift;
+}
+
 static XiveTCTX *pnv_xive2_get_indirect_tctx(PnvXive2 *xive, uint32_t pir)
 {
     PnvChip *chip = xive->chip;
@@ -1596,10 +1602,12 @@ static uint64_t pnv_xive2_ic_tm_indirect_read(void *opaque, hwaddr offset,
                                               unsigned size)
 {
     PnvXive2 *xive = PNV_XIVE2(opaque);
-    uint32_t pir = offset >> xive->ic_shift;
-    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
+    uint32_t pir;
+    XiveTCTX *tctx;
     uint64_t val = -1;
 
+    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
+    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
     if (tctx) {
         val = xive_tctx_tm_read(NULL, tctx, offset, size);
     }
@@ -1611,9 +1619,11 @@ static void pnv_xive2_ic_tm_indirect_write(void *opaque, hwaddr offset,
                                            uint64_t val, unsigned size)
 {
     PnvXive2 *xive = PNV_XIVE2(opaque);
-    uint32_t pir = offset >> xive->ic_shift;
-    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
+    uint32_t pir;
+    XiveTCTX *tctx;
 
+    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
+    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
     if (tctx) {
         xive_tctx_tm_write(NULL, tctx, offset, val, size);
     }
-- 
2.35.3

Re: [PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[Cédric Le Goater]

On 6/2/22 18:53, Frederic Barrat wrote:
> When accessing a thread context through the IC BAR, the offset of the
> page in the BAR identifies the CPU. From that offset, we can compute
> the PIR (processor ID register) of the CPU to do the data structure
> lookup. On P10, the current code assumes an access for node 0 when
> computing the PIR. Everything is almost in place to allow access for
> other nodes though. So this patch reworks how the PIR value is
> computed so that we can access all thread contexts through the IC BAR.
> 
> The PIR is already correct on P9, so no need to modify anything there.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Is that a P10 bug ? If so, a fixes tag is needed.

Thanks,

C.

> ---
>   hw/intc/pnv_xive2.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index a39e070e82..f31c53c28d 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1574,6 +1574,12 @@ static const MemoryRegionOps pnv_xive2_ic_sync_ops = {
>    * When the TM direct pages of the IC controller are accessed, the
>    * target HW thread is deduced from the page offset.
>    */
> +static uint32_t pnv_xive2_ic_tm_get_pir(PnvXive2 *xive, hwaddr offset)
> +{
> +    /* On P10, the node ID shift in the PIR register is 8 bits */
> +    return xive->chip->chip_id << 8 | offset >> xive->ic_shift;
> +}
> +
>   static XiveTCTX *pnv_xive2_get_indirect_tctx(PnvXive2 *xive, uint32_t pir)
>   {
>       PnvChip *chip = xive->chip;
> @@ -1596,10 +1602,12 @@ static uint64_t pnv_xive2_ic_tm_indirect_read(void *opaque, hwaddr offset,
>                                                 unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t pir = offset >> xive->ic_shift;
> -    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
> +    uint32_t pir;
> +    XiveTCTX *tctx;
>       uint64_t val = -1;
>   
> +    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
> +    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
>       if (tctx) {
>           val = xive_tctx_tm_read(NULL, tctx, offset, size);
>       }
> @@ -1611,9 +1619,11 @@ static void pnv_xive2_ic_tm_indirect_write(void *opaque, hwaddr offset,
>                                              uint64_t val, unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t pir = offset >> xive->ic_shift;
> -    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
> +    uint32_t pir;
> +    XiveTCTX *tctx;
>   
> +    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
> +    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
>       if (tctx) {
>           xive_tctx_tm_write(NULL, tctx, offset, val, size);
>       }

Re: [PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[Frederic Barrat]

On 02/06/2022 19:00, Cédric Le Goater wrote:
> On 6/2/22 18:53, Frederic Barrat wrote:
>> When accessing a thread context through the IC BAR, the offset of the
>> page in the BAR identifies the CPU. From that offset, we can compute
>> the PIR (processor ID register) of the CPU to do the data structure
>> lookup. On P10, the current code assumes an access for node 0 when
>> computing the PIR. Everything is almost in place to allow access for
>> other nodes though. So this patch reworks how the PIR value is
>> computed so that we can access all thread contexts through the IC BAR.
>>
>> The PIR is already correct on P9, so no need to modify anything there.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Is that a P10 bug ? If so, a fixes tag is needed.


Fixes: da71b7e3ed45 ("ppc/pnv: Add a XIVE2 controller to the POWER10 chip")

Daniel, good enough or you prefer a resend?

   Fred

Re: [PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[Daniel Henrique Barboza]

On 6/2/22 14:06, Frederic Barrat wrote:
> 
> 
> On 02/06/2022 19:00, Cédric Le Goater wrote:
>> On 6/2/22 18:53, Frederic Barrat wrote:
>>> When accessing a thread context through the IC BAR, the offset of the
>>> page in the BAR identifies the CPU. From that offset, we can compute
>>> the PIR (processor ID register) of the CPU to do the data structure
>>> lookup. On P10, the current code assumes an access for node 0 when
>>> computing the PIR. Everything is almost in place to allow access for
>>> other nodes though. So this patch reworks how the PIR value is
>>> computed so that we can access all thread contexts through the IC BAR.
>>>
>>> The PIR is already correct on P9, so no need to modify anything there.
>>>
>>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>
>> Is that a P10 bug ? If so, a fixes tag is needed.
> 
> 
> Fixes: da71b7e3ed45 ("ppc/pnv: Add a XIVE2 controller to the POWER10 chip")
> 
> Daniel, good enough or you prefer a resend?

I can fixup the tag, don't worry about it.


Daniel

> 
>    Fred

Re: [PATCH] pnv/xive2: Access direct mapped thread contexts from all chips

[Daniel Henrique Barboza]

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 6/2/22 13:53, Frederic Barrat wrote:
> When accessing a thread context through the IC BAR, the offset of the
> page in the BAR identifies the CPU. From that offset, we can compute
> the PIR (processor ID register) of the CPU to do the data structure
> lookup. On P10, the current code assumes an access for node 0 when
> computing the PIR. Everything is almost in place to allow access for
> other nodes though. So this patch reworks how the PIR value is
> computed so that we can access all thread contexts through the IC BAR.
> 
> The PIR is already correct on P9, so no need to modify anything there.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/intc/pnv_xive2.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index a39e070e82..f31c53c28d 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1574,6 +1574,12 @@ static const MemoryRegionOps pnv_xive2_ic_sync_ops = {
>    * When the TM direct pages of the IC controller are accessed, the
>    * target HW thread is deduced from the page offset.
>    */
> +static uint32_t pnv_xive2_ic_tm_get_pir(PnvXive2 *xive, hwaddr offset)
> +{
> +    /* On P10, the node ID shift in the PIR register is 8 bits */
> +    return xive->chip->chip_id << 8 | offset >> xive->ic_shift;
> +}
> +
>   static XiveTCTX *pnv_xive2_get_indirect_tctx(PnvXive2 *xive, uint32_t pir)
>   {
>       PnvChip *chip = xive->chip;
> @@ -1596,10 +1602,12 @@ static uint64_t pnv_xive2_ic_tm_indirect_read(void *opaque, hwaddr offset,
>                                                 unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t pir = offset >> xive->ic_shift;
> -    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
> +    uint32_t pir;
> +    XiveTCTX *tctx;
>       uint64_t val = -1;
>   
> +    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
> +    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
>       if (tctx) {
>           val = xive_tctx_tm_read(NULL, tctx, offset, size);
>       }
> @@ -1611,9 +1619,11 @@ static void pnv_xive2_ic_tm_indirect_write(void *opaque, hwaddr offset,
>                                              uint64_t val, unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t pir = offset >> xive->ic_shift;
> -    XiveTCTX *tctx = pnv_xive2_get_indirect_tctx(xive, pir);
> +    uint32_t pir;
> +    XiveTCTX *tctx;
>   
> +    pir = pnv_xive2_ic_tm_get_pir(xive, offset);
> +    tctx = pnv_xive2_get_indirect_tctx(xive, pir);
>       if (tctx) {
>           xive_tctx_tm_write(NULL, tctx, offset, val, size);
>       }