pnv/xive2: Don't overwrite PC registers when writing TCTXT registers

[PATCH] pnv/xive2: Don't overwrite PC registers when writing TCTXT registers

Posted by Frederic Barrat 3 years, 8 months ago

When writing a register from the TCTXT memory region (4th page within
the IC BAR), we were overwriting the Presentation Controller (PC)
register at the same offset. It looks like a silly cut and paste
error.

We were somehow lucky: the TCTXT registers being touched are
TCTXT_ENx/_SET/_RESET to enable physical threads and the PC registers
at the same offset are either not used by our model or the update was
harmless.

Found through code inspection.

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

diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index 87303b4064..a39e070e82 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -1295,7 +1295,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
                                      uint64_t val, unsigned size)
 {
     PnvXive2 *xive = PNV_XIVE2(opaque);
-    uint32_t reg = offset >> 3;
 
     switch (offset) {
     /*
@@ -1322,8 +1321,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
         xive2_error(xive, "TCTXT: invalid write @%"HWADDR_PRIx, offset);
         return;
     }
-
-    xive->pc_regs[reg] = val;
 }
 
 static const MemoryRegionOps pnv_xive2_ic_tctxt_ops = {
-- 
2.35.3

Re: [PATCH] pnv/xive2: Don't overwrite PC registers when writing TCTXT registers

Posted by Daniel Henrique Barboza 3 years, 8 months ago

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


Daniel

On 5/23/22 12:18, Frederic Barrat wrote:
> When writing a register from the TCTXT memory region (4th page within
> the IC BAR), we were overwriting the Presentation Controller (PC)
> register at the same offset. It looks like a silly cut and paste
> error.
> 
> We were somehow lucky: the TCTXT registers being touched are
> TCTXT_ENx/_SET/_RESET to enable physical threads and the PC registers
> at the same offset are either not used by our model or the update was
> harmless.
> 
> Found through code inspection.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/intc/pnv_xive2.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 87303b4064..a39e070e82 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1295,7 +1295,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
>                                        uint64_t val, unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t reg = offset >> 3;
>   
>       switch (offset) {
>       /*
> @@ -1322,8 +1321,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
>           xive2_error(xive, "TCTXT: invalid write @%"HWADDR_PRIx, offset);
>           return;
>       }
> -
> -    xive->pc_regs[reg] = val;
>   }
>   
>   static const MemoryRegionOps pnv_xive2_ic_tctxt_ops = {

Re: [PATCH] pnv/xive2: Don't overwrite PC registers when writing TCTXT registers

Posted by Cédric Le Goater 3 years, 8 months ago

On 5/23/22 17:18, Frederic Barrat wrote:
> When writing a register from the TCTXT memory region (4th page within
> the IC BAR), we were overwriting the Presentation Controller (PC)
> register at the same offset. It looks like a silly cut and paste
> error.

Very silly indeed :)

  
> We were somehow lucky: the TCTXT registers being touched are
> TCTXT_ENx/_SET/_RESET to enable physical threads and the PC registers
> at the same offset are either not used by our model or the update was
> harmless.
> 
> Found through code inspection.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>

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

Thanks,

C.

> ---
>   hw/intc/pnv_xive2.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 87303b4064..a39e070e82 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1295,7 +1295,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
>                                        uint64_t val, unsigned size)
>   {
>       PnvXive2 *xive = PNV_XIVE2(opaque);
> -    uint32_t reg = offset >> 3;
>   
>       switch (offset) {
>       /*
> @@ -1322,8 +1321,6 @@ static void pnv_xive2_ic_tctxt_write(void *opaque, hwaddr offset,
>           xive2_error(xive, "TCTXT: invalid write @%"HWADDR_PRIx, offset);
>           return;
>       }
> -
> -    xive->pc_regs[reg] = val;
>   }
>   
>   static const MemoryRegionOps pnv_xive2_ic_tctxt_ops = {