[PATCH 0/2] pnv/xive2: Fix TIMA special ops detection

[PATCH 0/2] pnv/xive2: Fix TIMA special ops detection

[Frederic Barrat]

Fix the TIMA special ops detection regression, as spotted by Coverity.

Tested by running a pseries guest on top of a powernv9 and powernv10 host.

Frederic Barrat (2):
  pnv/xive2: Add a get_config() method on the presenter class
  pnv/xive2: Check TIMA special ops against a dedicated array for P10

 hw/intc/pnv_xive.c    | 11 +++++++++
 hw/intc/pnv_xive2.c   | 44 +++++++++------------------------
 hw/intc/xive.c        | 57 +++++++++++++++++++++++++++++++++++++------
 include/hw/ppc/xive.h |  3 +++
 4 files changed, 75 insertions(+), 40 deletions(-)

-- 
2.41.0

[PATCH 1/2] pnv/xive2: Add a get_config() method on the presenter class

[Frederic Barrat]

The presenters for xive on P9 and P10 are mostly similar but the
behavior can be tuned through a few CQ registers. This patch adds a
"get_config" method, which will allow to access that config from the
presenter in a later patch.
For now, just define the config for the TIMA version.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/intc/pnv_xive.c    | 11 +++++++++++
 hw/intc/pnv_xive2.c   | 12 ++++++++++++
 hw/intc/xive.c        |  7 +++++++
 include/hw/ppc/xive.h |  3 +++
 4 files changed, 33 insertions(+)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 622f9d28b7..e536b3ec26 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -479,6 +479,16 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
     return count;
 }
 
+static uint32_t pnv_xive_presenter_get_config(XivePresenter *xptr)
+{
+    uint32_t cfg = 0;
+
+    /* TIMA GEN1 is all P9 knows */
+    cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
+
+    return cfg;
+}
+
 static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
 {
     return pnv_xive_block_id(PNV_XIVE(xrtr));
@@ -1991,6 +2001,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
 
     xnc->notify = pnv_xive_notify;
     xpc->match_nvt  = pnv_xive_match_nvt;
+    xpc->get_config = pnv_xive_presenter_get_config;
 };
 
 static const TypeInfo pnv_xive_info = {
diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index ec1edeb385..59534f6843 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -501,6 +501,17 @@ static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
     return count;
 }
 
+static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
+{
+    PnvXive2 *xive = PNV_XIVE2(xptr);
+    uint32_t cfg = 0;
+
+    if (xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS) {
+        cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
+    }
+    return cfg;
+}
+
 static uint8_t pnv_xive2_get_block_id(Xive2Router *xrtr)
 {
     return pnv_xive2_block_id(PNV_XIVE2(xrtr));
@@ -1987,6 +1998,7 @@ static void pnv_xive2_class_init(ObjectClass *klass, void *data)
     xnc->notify    = pnv_xive2_notify;
 
     xpc->match_nvt  = pnv_xive2_match_nvt;
+    xpc->get_config = pnv_xive2_presenter_get_config;
 };
 
 static const TypeInfo pnv_xive2_info = {
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 5204c14b87..34a868b185 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -461,6 +461,13 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
     }
 }
 
+static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
+{
+    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
+
+    return xpc->get_config(xptr);
+}
+
 /*
  * Define a mapping of "special" operations depending on the TIMA page
  * offset and the size of the operation.
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index f7eea4ca81..3dfb06e002 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -430,6 +430,8 @@ typedef struct XivePresenterClass XivePresenterClass;
 DECLARE_CLASS_CHECKERS(XivePresenterClass, XIVE_PRESENTER,
                        TYPE_XIVE_PRESENTER)
 
+#define XIVE_PRESENTER_GEN1_TIMA_OS     0x1
+
 struct XivePresenterClass {
     InterfaceClass parent;
     int (*match_nvt)(XivePresenter *xptr, uint8_t format,
@@ -437,6 +439,7 @@ struct XivePresenterClass {
                      bool cam_ignore, uint8_t priority,
                      uint32_t logic_serv, XiveTCTXMatch *match);
     bool (*in_kernel)(const XivePresenter *xptr);
+    uint32_t (*get_config)(XivePresenter *xptr);
 };
 
 int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
-- 
2.41.0

Re: [PATCH 1/2] pnv/xive2: Add a get_config() method on the presenter class

[Cédric Le Goater]

On 6/21/23 18:03, Frederic Barrat wrote:
> The presenters for xive on P9 and P10 are mostly similar but the
> behavior can be tuned through a few CQ registers. This patch adds a
> "get_config" method, which will allow to access that config from the
> presenter in a later patch.
> For now, just define the config for the TIMA version.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>

Looks good. If I remember well, each XIVE subunit has a copy of the
config registers and modifications to the CQ unit are spanned to the
others.

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

Thanks,

C.

> ---
>   hw/intc/pnv_xive.c    | 11 +++++++++++
>   hw/intc/pnv_xive2.c   | 12 ++++++++++++
>   hw/intc/xive.c        |  7 +++++++
>   include/hw/ppc/xive.h |  3 +++
>   4 files changed, 33 insertions(+)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 622f9d28b7..e536b3ec26 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -479,6 +479,16 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>       return count;
>   }
>   
> +static uint32_t pnv_xive_presenter_get_config(XivePresenter *xptr)
> +{
> +    uint32_t cfg = 0;
> +
> +    /* TIMA GEN1 is all P9 knows */
> +    cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
> +
> +    return cfg;
> +}
> +
>   static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
>   {
>       return pnv_xive_block_id(PNV_XIVE(xrtr));
> @@ -1991,6 +2001,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
>   
>       xnc->notify = pnv_xive_notify;
>       xpc->match_nvt  = pnv_xive_match_nvt;
> +    xpc->get_config = pnv_xive_presenter_get_config;
>   };
>   
>   static const TypeInfo pnv_xive_info = {
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index ec1edeb385..59534f6843 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -501,6 +501,17 @@ static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
>       return count;
>   }
>   
> +static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
> +{
> +    PnvXive2 *xive = PNV_XIVE2(xptr);
> +    uint32_t cfg = 0;
> +
> +    if (xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS) {
> +        cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
> +    }
> +    return cfg;
> +}
> +
>   static uint8_t pnv_xive2_get_block_id(Xive2Router *xrtr)
>   {
>       return pnv_xive2_block_id(PNV_XIVE2(xrtr));
> @@ -1987,6 +1998,7 @@ static void pnv_xive2_class_init(ObjectClass *klass, void *data)
>       xnc->notify    = pnv_xive2_notify;
>   
>       xpc->match_nvt  = pnv_xive2_match_nvt;
> +    xpc->get_config = pnv_xive2_presenter_get_config;
>   };
>   
>   static const TypeInfo pnv_xive2_info = {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 5204c14b87..34a868b185 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -461,6 +461,13 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>       }
>   }
>   
> +static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
> +{
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +
> +    return xpc->get_config(xptr);
> +}
> +
>   /*
>    * Define a mapping of "special" operations depending on the TIMA page
>    * offset and the size of the operation.
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index f7eea4ca81..3dfb06e002 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -430,6 +430,8 @@ typedef struct XivePresenterClass XivePresenterClass;
>   DECLARE_CLASS_CHECKERS(XivePresenterClass, XIVE_PRESENTER,
>                          TYPE_XIVE_PRESENTER)
>   
> +#define XIVE_PRESENTER_GEN1_TIMA_OS     0x1
> +
>   struct XivePresenterClass {
>       InterfaceClass parent;
>       int (*match_nvt)(XivePresenter *xptr, uint8_t format,
> @@ -437,6 +439,7 @@ struct XivePresenterClass {
>                        bool cam_ignore, uint8_t priority,
>                        uint32_t logic_serv, XiveTCTXMatch *match);
>       bool (*in_kernel)(const XivePresenter *xptr);
> +    uint32_t (*get_config)(XivePresenter *xptr);
>   };
>   
>   int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,

Re: [PATCH 1/2] pnv/xive2: Add a get_config() method on the presenter class

[Cédric Le Goater]

On 6/21/23 18:03, Frederic Barrat wrote:
> The presenters for xive on P9 and P10 are mostly similar but the
> behavior can be tuned through a few CQ registers. This patch adds a
> "get_config" method, which will allow to access that config from the
> presenter in a later patch.
> For now, just define the config for the TIMA version.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/intc/pnv_xive.c    | 11 +++++++++++
>   hw/intc/pnv_xive2.c   | 12 ++++++++++++

spapr_xive.c needs an update too else QEMU will SEGV at first interrupt.

Thanks,

C.

>   hw/intc/xive.c        |  7 +++++++
>   include/hw/ppc/xive.h |  3 +++
>   4 files changed, 33 insertions(+)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 622f9d28b7..e536b3ec26 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -479,6 +479,16 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>       return count;
>   }
>   
> +static uint32_t pnv_xive_presenter_get_config(XivePresenter *xptr)
> +{
> +    uint32_t cfg = 0;
> +
> +    /* TIMA GEN1 is all P9 knows */
> +    cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
> +
> +    return cfg;
> +}
> +
>   static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
>   {
>       return pnv_xive_block_id(PNV_XIVE(xrtr));
> @@ -1991,6 +2001,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
>   
>       xnc->notify = pnv_xive_notify;
>       xpc->match_nvt  = pnv_xive_match_nvt;
> +    xpc->get_config = pnv_xive_presenter_get_config;
>   };
>   
>   static const TypeInfo pnv_xive_info = {
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index ec1edeb385..59534f6843 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -501,6 +501,17 @@ static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
>       return count;
>   }
>   
> +static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
> +{
> +    PnvXive2 *xive = PNV_XIVE2(xptr);
> +    uint32_t cfg = 0;
> +
> +    if (xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS) {
> +        cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
> +    }
> +    return cfg;
> +}
> +
>   static uint8_t pnv_xive2_get_block_id(Xive2Router *xrtr)
>   {
>       return pnv_xive2_block_id(PNV_XIVE2(xrtr));
> @@ -1987,6 +1998,7 @@ static void pnv_xive2_class_init(ObjectClass *klass, void *data)
>       xnc->notify    = pnv_xive2_notify;
>   
>       xpc->match_nvt  = pnv_xive2_match_nvt;
> +    xpc->get_config = pnv_xive2_presenter_get_config;
>   };
>   
>   static const TypeInfo pnv_xive2_info = {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 5204c14b87..34a868b185 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -461,6 +461,13 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>       }
>   }
>   
> +static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
> +{
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +
> +    return xpc->get_config(xptr);
> +}
> +
>   /*
>    * Define a mapping of "special" operations depending on the TIMA page
>    * offset and the size of the operation.
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index f7eea4ca81..3dfb06e002 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -430,6 +430,8 @@ typedef struct XivePresenterClass XivePresenterClass;
>   DECLARE_CLASS_CHECKERS(XivePresenterClass, XIVE_PRESENTER,
>                          TYPE_XIVE_PRESENTER)
>   
> +#define XIVE_PRESENTER_GEN1_TIMA_OS     0x1
> +
>   struct XivePresenterClass {
>       InterfaceClass parent;
>       int (*match_nvt)(XivePresenter *xptr, uint8_t format,
> @@ -437,6 +439,7 @@ struct XivePresenterClass {
>                        bool cam_ignore, uint8_t priority,
>                        uint32_t logic_serv, XiveTCTXMatch *match);
>       bool (*in_kernel)(const XivePresenter *xptr);
> +    uint32_t (*get_config)(XivePresenter *xptr);
>   };
>   
>   int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,

Re: [PATCH 1/2] pnv/xive2: Add a get_config() method on the presenter class

[Frederic Barrat]

On 22/06/2023 09:01, Cédric Le Goater wrote:
> On 6/21/23 18:03, Frederic Barrat wrote:
>> The presenters for xive on P9 and P10 are mostly similar but the
>> behavior can be tuned through a few CQ registers. This patch adds a
>> "get_config" method, which will allow to access that config from the
>> presenter in a later patch.
>> For now, just define the config for the TIMA version.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>> ---
>>   hw/intc/pnv_xive.c    | 11 +++++++++++
>>   hw/intc/pnv_xive2.c   | 12 ++++++++++++
> 
> spapr_xive.c needs an update too else QEMU will SEGV at first interrupt.


Sigh... I should really start using your qemu-ppc-boot tests.

   Fred

> 
> Thanks,
> 
> C.
> 
>>   hw/intc/xive.c        |  7 +++++++
>>   include/hw/ppc/xive.h |  3 +++
>>   4 files changed, 33 insertions(+)
>>
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 622f9d28b7..e536b3ec26 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -479,6 +479,16 @@ static int pnv_xive_match_nvt(XivePresenter 
>> *xptr, uint8_t format,
>>       return count;
>>   }
>> +static uint32_t pnv_xive_presenter_get_config(XivePresenter *xptr)
>> +{
>> +    uint32_t cfg = 0;
>> +
>> +    /* TIMA GEN1 is all P9 knows */
>> +    cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
>> +
>> +    return cfg;
>> +}
>> +
>>   static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
>>   {
>>       return pnv_xive_block_id(PNV_XIVE(xrtr));
>> @@ -1991,6 +2001,7 @@ static void pnv_xive_class_init(ObjectClass 
>> *klass, void *data)
>>       xnc->notify = pnv_xive_notify;
>>       xpc->match_nvt  = pnv_xive_match_nvt;
>> +    xpc->get_config = pnv_xive_presenter_get_config;
>>   };
>>   static const TypeInfo pnv_xive_info = {
>> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
>> index ec1edeb385..59534f6843 100644
>> --- a/hw/intc/pnv_xive2.c
>> +++ b/hw/intc/pnv_xive2.c
>> @@ -501,6 +501,17 @@ static int pnv_xive2_match_nvt(XivePresenter 
>> *xptr, uint8_t format,
>>       return count;
>>   }
>> +static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
>> +{
>> +    PnvXive2 *xive = PNV_XIVE2(xptr);
>> +    uint32_t cfg = 0;
>> +
>> +    if (xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS) {
>> +        cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
>> +    }
>> +    return cfg;
>> +}
>> +
>>   static uint8_t pnv_xive2_get_block_id(Xive2Router *xrtr)
>>   {
>>       return pnv_xive2_block_id(PNV_XIVE2(xrtr));
>> @@ -1987,6 +1998,7 @@ static void pnv_xive2_class_init(ObjectClass 
>> *klass, void *data)
>>       xnc->notify    = pnv_xive2_notify;
>>       xpc->match_nvt  = pnv_xive2_match_nvt;
>> +    xpc->get_config = pnv_xive2_presenter_get_config;
>>   };
>>   static const TypeInfo pnv_xive2_info = {
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 5204c14b87..34a868b185 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -461,6 +461,13 @@ static void xive_tm_push_os_ctx(XivePresenter 
>> *xptr, XiveTCTX *tctx,
>>       }
>>   }
>> +static __attribute__((unused)) uint32_t 
>> xive_presenter_get_config(XivePresenter *xptr)
>> +{
>> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>> +
>> +    return xpc->get_config(xptr);
>> +}
>> +
>>   /*
>>    * Define a mapping of "special" operations depending on the TIMA page
>>    * offset and the size of the operation.
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index f7eea4ca81..3dfb06e002 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -430,6 +430,8 @@ typedef struct XivePresenterClass XivePresenterClass;
>>   DECLARE_CLASS_CHECKERS(XivePresenterClass, XIVE_PRESENTER,
>>                          TYPE_XIVE_PRESENTER)
>> +#define XIVE_PRESENTER_GEN1_TIMA_OS     0x1
>> +
>>   struct XivePresenterClass {
>>       InterfaceClass parent;
>>       int (*match_nvt)(XivePresenter *xptr, uint8_t format,
>> @@ -437,6 +439,7 @@ struct XivePresenterClass {
>>                        bool cam_ignore, uint8_t priority,
>>                        uint32_t logic_serv, XiveTCTXMatch *match);
>>       bool (*in_kernel)(const XivePresenter *xptr);
>> +    uint32_t (*get_config)(XivePresenter *xptr);
>>   };
>>   int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
> 

Re: [PATCH 1/2] pnv/xive2: Add a get_config() method on the presenter class

[Cédric Le Goater]

On 6/22/23 09:53, Frederic Barrat wrote:
> 
> 
> On 22/06/2023 09:01, Cédric Le Goater wrote:
>> On 6/21/23 18:03, Frederic Barrat wrote:
>>> The presenters for xive on P9 and P10 are mostly similar but the
>>> behavior can be tuned through a few CQ registers. This patch adds a
>>> "get_config" method, which will allow to access that config from the
>>> presenter in a later patch.
>>> For now, just define the config for the TIMA version.
>>>
>>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>>> ---
>>>   hw/intc/pnv_xive.c    | 11 +++++++++++
>>>   hw/intc/pnv_xive2.c   | 12 ++++++++++++
>>
>> spapr_xive.c needs an update too else QEMU will SEGV at first interrupt.
> 
> 
> Sigh... I should really start using your qemu-ppc-boot tests.

yes. That's how I caugh it.

QEMU has an avocado test :

   build/tests/venv/bin/avocado -t machine:pseries  build/tests/avocado/boot_linux.py

which hangs. More tests would be  good to have.

Thanks,

C.

> 
>    Fred
> 
>>
>> Thanks,
>>
>> C.
>>
>>>   hw/intc/xive.c        |  7 +++++++
>>>   include/hw/ppc/xive.h |  3 +++
>>>   4 files changed, 33 insertions(+)
>>>
>>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>>> index 622f9d28b7..e536b3ec26 100644
>>> --- a/hw/intc/pnv_xive.c
>>> +++ b/hw/intc/pnv_xive.c
>>> @@ -479,6 +479,16 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>>>       return count;
>>>   }
>>> +static uint32_t pnv_xive_presenter_get_config(XivePresenter *xptr)
>>> +{
>>> +    uint32_t cfg = 0;
>>> +
>>> +    /* TIMA GEN1 is all P9 knows */
>>> +    cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
>>> +
>>> +    return cfg;
>>> +}
>>> +
>>>   static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
>>>   {
>>>       return pnv_xive_block_id(PNV_XIVE(xrtr));
>>> @@ -1991,6 +2001,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
>>>       xnc->notify = pnv_xive_notify;
>>>       xpc->match_nvt  = pnv_xive_match_nvt;
>>> +    xpc->get_config = pnv_xive_presenter_get_config;
>>>   };
>>>   static const TypeInfo pnv_xive_info = {
>>> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
>>> index ec1edeb385..59534f6843 100644
>>> --- a/hw/intc/pnv_xive2.c
>>> +++ b/hw/intc/pnv_xive2.c
>>> @@ -501,6 +501,17 @@ static int pnv_xive2_match_nvt(XivePresenter *xptr, uint8_t format,
>>>       return count;
>>>   }
>>> +static uint32_t pnv_xive2_presenter_get_config(XivePresenter *xptr)
>>> +{
>>> +    PnvXive2 *xive = PNV_XIVE2(xptr);
>>> +    uint32_t cfg = 0;
>>> +
>>> +    if (xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS) {
>>> +        cfg |= XIVE_PRESENTER_GEN1_TIMA_OS;
>>> +    }
>>> +    return cfg;
>>> +}
>>> +
>>>   static uint8_t pnv_xive2_get_block_id(Xive2Router *xrtr)
>>>   {
>>>       return pnv_xive2_block_id(PNV_XIVE2(xrtr));
>>> @@ -1987,6 +1998,7 @@ static void pnv_xive2_class_init(ObjectClass *klass, void *data)
>>>       xnc->notify    = pnv_xive2_notify;
>>>       xpc->match_nvt  = pnv_xive2_match_nvt;
>>> +    xpc->get_config = pnv_xive2_presenter_get_config;
>>>   };
>>>   static const TypeInfo pnv_xive2_info = {
>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>>> index 5204c14b87..34a868b185 100644
>>> --- a/hw/intc/xive.c
>>> +++ b/hw/intc/xive.c
>>> @@ -461,6 +461,13 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>>>       }
>>>   }
>>> +static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
>>> +{
>>> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>>> +
>>> +    return xpc->get_config(xptr);
>>> +}
>>> +
>>>   /*
>>>    * Define a mapping of "special" operations depending on the TIMA page
>>>    * offset and the size of the operation.
>>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>>> index f7eea4ca81..3dfb06e002 100644
>>> --- a/include/hw/ppc/xive.h
>>> +++ b/include/hw/ppc/xive.h
>>> @@ -430,6 +430,8 @@ typedef struct XivePresenterClass XivePresenterClass;
>>>   DECLARE_CLASS_CHECKERS(XivePresenterClass, XIVE_PRESENTER,
>>>                          TYPE_XIVE_PRESENTER)
>>> +#define XIVE_PRESENTER_GEN1_TIMA_OS     0x1
>>> +
>>>   struct XivePresenterClass {
>>>       InterfaceClass parent;
>>>       int (*match_nvt)(XivePresenter *xptr, uint8_t format,
>>> @@ -437,6 +439,7 @@ struct XivePresenterClass {
>>>                        bool cam_ignore, uint8_t priority,
>>>                        uint32_t logic_serv, XiveTCTXMatch *match);
>>>       bool (*in_kernel)(const XivePresenter *xptr);
>>> +    uint32_t (*get_config)(XivePresenter *xptr);
>>>   };
>>>   int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>>

[PATCH 2/2] pnv/xive2: Check TIMA special ops against a dedicated array for P10

[Frederic Barrat]

Accessing the TIMA from some specific ring/offset combination can
trigger a special operation, with or without side effects. It is
implemented in qemu with an array of special operations to compare
accesses against. Since the presenter on P10 is pretty similar to P9,
we had the full array defined for P9 and we just had a special case
for P10 to treat one access differently. With a recent change,
6f2cbd133d4 ("pnv/xive2: Handle TIMA access through all ports"), we
now ignore some of the bits of the TIMA address, but that patch
managed to botch the detection of the special case for P10.

To clean that up, this patch introduces a full array of special ops to
be used for P10. The code to detect a special access is common with
P9, only the array of operations differs. The presenter can pick the
correct array of special ops based on its configuration introduced in
a previous patch.

Fixes: Coverity CID 1512997, 1512998
Fixes: 6f2cbd133d4 ("pnv/xive2: Handle TIMA access through all ports")
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/intc/pnv_xive2.c | 32 ----------------------------
 hw/intc/xive.c      | 52 +++++++++++++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index 59534f6843..ed438a20ed 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -1656,17 +1656,6 @@ static const MemoryRegionOps pnv_xive2_ic_tm_indirect_ops = {
 /*
  * TIMA ops
  */
-
-/*
- * Special TIMA offsets to handle accesses in a POWER10 way.
- *
- * Only the CAM line updates done by the hypervisor should be handled
- * specifically.
- */
-#define HV_PAGE_OFFSET         (XIVE_TM_HV_PAGE << TM_SHIFT)
-#define HV_PUSH_OS_CTX_OFFSET  (HV_PAGE_OFFSET | (TM_QW1_OS + TM_WORD2))
-#define HV_PULL_OS_CTX_OFFSET  (HV_PAGE_OFFSET | TM_SPC_PULL_OS_CTX)
-
 static void pnv_xive2_tm_write(void *opaque, hwaddr offset,
                                uint64_t value, unsigned size)
 {
@@ -1674,18 +1663,7 @@ static void pnv_xive2_tm_write(void *opaque, hwaddr offset,
     PnvXive2 *xive = pnv_xive2_tm_get_xive(cpu);
     XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
     XivePresenter *xptr = XIVE_PRESENTER(xive);
-    bool gen1_tima_os =
-        xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS;
-
-    offset &= TM_ADDRESS_MASK;
 
-    /* TODO: should we switch the TM ops table instead ? */
-    if (!gen1_tima_os && offset == HV_PUSH_OS_CTX_OFFSET) {
-        xive2_tm_push_os_ctx(xptr, tctx, offset, value, size);
-        return;
-    }
-
-    /* Other TM ops are the same as XIVE1 */
     xive_tctx_tm_write(xptr, tctx, offset, value, size);
 }
 
@@ -1695,17 +1673,7 @@ static uint64_t pnv_xive2_tm_read(void *opaque, hwaddr offset, unsigned size)
     PnvXive2 *xive = pnv_xive2_tm_get_xive(cpu);
     XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
     XivePresenter *xptr = XIVE_PRESENTER(xive);
-    bool gen1_tima_os =
-        xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS;
-
-    offset &= TM_ADDRESS_MASK;
-
-    /* TODO: should we switch the TM ops table instead ? */
-    if (!gen1_tima_os && offset == HV_PULL_OS_CTX_OFFSET) {
-        return xive2_tm_pull_os_ctx(xptr, tctx, offset, size);
-    }
 
-    /* Other TM ops are the same as XIVE1 */
     return xive_tctx_tm_read(xptr, tctx, offset, size);
 }
 
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 34a868b185..84c079b034 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -20,6 +20,7 @@
 #include "monitor/monitor.h"
 #include "hw/irq.h"
 #include "hw/ppc/xive.h"
+#include "hw/ppc/xive2.h"
 #include "hw/ppc/xive_regs.h"
 #include "trace.h"
 
@@ -461,7 +462,7 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
     }
 }
 
-static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
+static uint32_t xive_presenter_get_config(XivePresenter *xptr)
 {
     XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
 
@@ -504,14 +505,47 @@ static const XiveTmOp xive_tm_operations[] = {
     { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  8, NULL, xive_tm_pull_pool_ctx },
 };
 
-static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
+static const XiveTmOp xive2_tm_operations[] = {
+    /*
+     * MMIOs below 2K : raw values and special operations without side
+     * effects
+     */
+    { XIVE_TM_OS_PAGE, TM_QW1_OS + TM_CPPR,   1, xive_tm_set_os_cppr, NULL },
+    { XIVE_TM_HV_PAGE, TM_QW1_OS + TM_WORD2,  4, xive2_tm_push_os_ctx, NULL },
+    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_CPPR, 1, xive_tm_set_hv_cppr, NULL },
+    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, xive_tm_vt_push, NULL },
+    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, NULL, xive_tm_vt_poll },
+
+    /* MMIOs above 2K : special operations with side effects */
+    { XIVE_TM_OS_PAGE, TM_SPC_ACK_OS_REG,     2, NULL, xive_tm_ack_os_reg },
+    { XIVE_TM_OS_PAGE, TM_SPC_SET_OS_PENDING, 1, xive_tm_set_os_pending, NULL },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    4, NULL, xive2_tm_pull_os_ctx },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    8, NULL, xive2_tm_pull_os_ctx },
+    { XIVE_TM_HV_PAGE, TM_SPC_ACK_HV_REG,     2, NULL, xive_tm_ack_hv_reg },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  4, NULL, xive_tm_pull_pool_ctx },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  8, NULL, xive_tm_pull_pool_ctx },
+};
+
+static const XiveTmOp *xive_tm_find_op(XivePresenter *xptr, hwaddr offset,
+                                       unsigned size, bool write)
 {
     uint8_t page_offset = (offset >> TM_SHIFT) & 0x3;
     uint32_t op_offset = offset & TM_ADDRESS_MASK;
-    int i;
+    const XiveTmOp *tm_ops;
+    int i, tm_ops_count;
+    uint32_t cfg;
+
+    cfg = xive_presenter_get_config(xptr);
+    if (cfg & XIVE_PRESENTER_GEN1_TIMA_OS) {
+        tm_ops = xive_tm_operations;
+        tm_ops_count = ARRAY_SIZE(xive_tm_operations);
+    } else {
+        tm_ops = xive2_tm_operations;
+        tm_ops_count = ARRAY_SIZE(xive2_tm_operations);
+    }
 
-    for (i = 0; i < ARRAY_SIZE(xive_tm_operations); i++) {
-        const XiveTmOp *xto = &xive_tm_operations[i];
+    for (i = 0; i < tm_ops_count; i++) {
+        const XiveTmOp *xto = &tm_ops[i];
 
         /* Accesses done from a more privileged TIMA page is allowed */
         if (xto->page_offset >= page_offset &&
@@ -542,7 +576,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
      * First, check for special operations in the 2K region
      */
     if (offset & TM_SPECIAL_OP) {
-        xto = xive_tm_find_op(offset, size, true);
+        xto = xive_tm_find_op(tctx->xptr, offset, size, true);
         if (!xto) {
             qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA "
                           "@%"HWADDR_PRIx"\n", offset);
@@ -555,7 +589,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
     /*
      * Then, for special operations in the region below 2K.
      */
-    xto = xive_tm_find_op(offset, size, true);
+    xto = xive_tm_find_op(tctx->xptr, offset, size, true);
     if (xto) {
         xto->write_handler(xptr, tctx, offset, value, size);
         return;
@@ -581,7 +615,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
      * First, check for special operations in the 2K region
      */
     if (offset & TM_SPECIAL_OP) {
-        xto = xive_tm_find_op(offset, size, false);
+        xto = xive_tm_find_op(tctx->xptr, offset, size, false);
         if (!xto) {
             qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid read access to TIMA"
                           "@%"HWADDR_PRIx"\n", offset);
@@ -594,7 +628,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
     /*
      * Then, for special operations in the region below 2K.
      */
-    xto = xive_tm_find_op(offset, size, false);
+    xto = xive_tm_find_op(tctx->xptr, offset, size, false);
     if (xto) {
         ret = xto->read_handler(xptr, tctx, offset, size);
         goto out;
-- 
2.41.0

Re: [PATCH 2/2] pnv/xive2: Check TIMA special ops against a dedicated array for P10

[Cédric Le Goater]

On 6/21/23 18:03, Frederic Barrat wrote:
> Accessing the TIMA from some specific ring/offset combination can
> trigger a special operation, with or without side effects. It is
> implemented in qemu with an array of special operations to compare
> accesses against. Since the presenter on P10 is pretty similar to P9,
> we had the full array defined for P9 and we just had a special case
> for P10 to treat one access differently. With a recent change,
> 6f2cbd133d4 ("pnv/xive2: Handle TIMA access through all ports"), we
> now ignore some of the bits of the TIMA address, but that patch
> managed to botch the detection of the special case for P10.
> 
> To clean that up, this patch introduces a full array of special ops to
> be used for P10. The code to detect a special access is common with
> P9, only the array of operations differs. The presenter can pick the
> correct array of special ops based on its configuration introduced in
> a previous patch.
> 
> Fixes: Coverity CID 1512997, 1512998
> Fixes: 6f2cbd133d4 ("pnv/xive2: Handle TIMA access through all ports")
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/intc/pnv_xive2.c | 32 ----------------------------
>   hw/intc/xive.c      | 52 +++++++++++++++++++++++++++++++++++++--------
>   2 files changed, 43 insertions(+), 41 deletions(-)

Not a big change after all.

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

Thanks,

C.


> 
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 59534f6843..ed438a20ed 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -1656,17 +1656,6 @@ static const MemoryRegionOps pnv_xive2_ic_tm_indirect_ops = {
>   /*
>    * TIMA ops
>    */
> -
> -/*
> - * Special TIMA offsets to handle accesses in a POWER10 way.
> - *
> - * Only the CAM line updates done by the hypervisor should be handled
> - * specifically.
> - */
> -#define HV_PAGE_OFFSET         (XIVE_TM_HV_PAGE << TM_SHIFT)
> -#define HV_PUSH_OS_CTX_OFFSET  (HV_PAGE_OFFSET | (TM_QW1_OS + TM_WORD2))
> -#define HV_PULL_OS_CTX_OFFSET  (HV_PAGE_OFFSET | TM_SPC_PULL_OS_CTX)
> -
>   static void pnv_xive2_tm_write(void *opaque, hwaddr offset,
>                                  uint64_t value, unsigned size)
>   {
> @@ -1674,18 +1663,7 @@ static void pnv_xive2_tm_write(void *opaque, hwaddr offset,
>       PnvXive2 *xive = pnv_xive2_tm_get_xive(cpu);
>       XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>       XivePresenter *xptr = XIVE_PRESENTER(xive);
> -    bool gen1_tima_os =
> -        xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS;
> -
> -    offset &= TM_ADDRESS_MASK;
>   
> -    /* TODO: should we switch the TM ops table instead ? */
> -    if (!gen1_tima_os && offset == HV_PUSH_OS_CTX_OFFSET) {
> -        xive2_tm_push_os_ctx(xptr, tctx, offset, value, size);
> -        return;
> -    }
> -
> -    /* Other TM ops are the same as XIVE1 */
>       xive_tctx_tm_write(xptr, tctx, offset, value, size);
>   }
>   
> @@ -1695,17 +1673,7 @@ static uint64_t pnv_xive2_tm_read(void *opaque, hwaddr offset, unsigned size)
>       PnvXive2 *xive = pnv_xive2_tm_get_xive(cpu);
>       XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>       XivePresenter *xptr = XIVE_PRESENTER(xive);
> -    bool gen1_tima_os =
> -        xive->cq_regs[CQ_XIVE_CFG >> 3] & CQ_XIVE_CFG_GEN1_TIMA_OS;
> -
> -    offset &= TM_ADDRESS_MASK;
> -
> -    /* TODO: should we switch the TM ops table instead ? */
> -    if (!gen1_tima_os && offset == HV_PULL_OS_CTX_OFFSET) {
> -        return xive2_tm_pull_os_ctx(xptr, tctx, offset, size);
> -    }
>   
> -    /* Other TM ops are the same as XIVE1 */
>       return xive_tctx_tm_read(xptr, tctx, offset, size);
>   }
>   
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 34a868b185..84c079b034 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -20,6 +20,7 @@
>   #include "monitor/monitor.h"
>   #include "hw/irq.h"
>   #include "hw/ppc/xive.h"
> +#include "hw/ppc/xive2.h"
>   #include "hw/ppc/xive_regs.h"
>   #include "trace.h"
>   
> @@ -461,7 +462,7 @@ static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>       }
>   }
>   
> -static __attribute__((unused)) uint32_t xive_presenter_get_config(XivePresenter *xptr)
> +static uint32_t xive_presenter_get_config(XivePresenter *xptr)
>   {
>       XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>   
> @@ -504,14 +505,47 @@ static const XiveTmOp xive_tm_operations[] = {
>       { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  8, NULL, xive_tm_pull_pool_ctx },
>   };
>   
> -static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
> +static const XiveTmOp xive2_tm_operations[] = {
> +    /*
> +     * MMIOs below 2K : raw values and special operations without side
> +     * effects
> +     */
> +    { XIVE_TM_OS_PAGE, TM_QW1_OS + TM_CPPR,   1, xive_tm_set_os_cppr, NULL },
> +    { XIVE_TM_HV_PAGE, TM_QW1_OS + TM_WORD2,  4, xive2_tm_push_os_ctx, NULL },
> +    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_CPPR, 1, xive_tm_set_hv_cppr, NULL },
> +    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, xive_tm_vt_push, NULL },
> +    { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, NULL, xive_tm_vt_poll },
> +
> +    /* MMIOs above 2K : special operations with side effects */
> +    { XIVE_TM_OS_PAGE, TM_SPC_ACK_OS_REG,     2, NULL, xive_tm_ack_os_reg },
> +    { XIVE_TM_OS_PAGE, TM_SPC_SET_OS_PENDING, 1, xive_tm_set_os_pending, NULL },
> +    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    4, NULL, xive2_tm_pull_os_ctx },
> +    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    8, NULL, xive2_tm_pull_os_ctx },
> +    { XIVE_TM_HV_PAGE, TM_SPC_ACK_HV_REG,     2, NULL, xive_tm_ack_hv_reg },
> +    { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  4, NULL, xive_tm_pull_pool_ctx },
> +    { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  8, NULL, xive_tm_pull_pool_ctx },
> +};
> +
> +static const XiveTmOp *xive_tm_find_op(XivePresenter *xptr, hwaddr offset,
> +                                       unsigned size, bool write)
>   {
>       uint8_t page_offset = (offset >> TM_SHIFT) & 0x3;
>       uint32_t op_offset = offset & TM_ADDRESS_MASK;
> -    int i;
> +    const XiveTmOp *tm_ops;
> +    int i, tm_ops_count;
> +    uint32_t cfg;
> +
> +    cfg = xive_presenter_get_config(xptr);
> +    if (cfg & XIVE_PRESENTER_GEN1_TIMA_OS) {
> +        tm_ops = xive_tm_operations;
> +        tm_ops_count = ARRAY_SIZE(xive_tm_operations);
> +    } else {
> +        tm_ops = xive2_tm_operations;
> +        tm_ops_count = ARRAY_SIZE(xive2_tm_operations);
> +    }
>   
> -    for (i = 0; i < ARRAY_SIZE(xive_tm_operations); i++) {
> -        const XiveTmOp *xto = &xive_tm_operations[i];
> +    for (i = 0; i < tm_ops_count; i++) {
> +        const XiveTmOp *xto = &tm_ops[i];
>   
>           /* Accesses done from a more privileged TIMA page is allowed */
>           if (xto->page_offset >= page_offset &&
> @@ -542,7 +576,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>        * First, check for special operations in the 2K region
>        */
>       if (offset & TM_SPECIAL_OP) {
> -        xto = xive_tm_find_op(offset, size, true);
> +        xto = xive_tm_find_op(tctx->xptr, offset, size, true);
>           if (!xto) {
>               qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA "
>                             "@%"HWADDR_PRIx"\n", offset);
> @@ -555,7 +589,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>       /*
>        * Then, for special operations in the region below 2K.
>        */
> -    xto = xive_tm_find_op(offset, size, true);
> +    xto = xive_tm_find_op(tctx->xptr, offset, size, true);
>       if (xto) {
>           xto->write_handler(xptr, tctx, offset, value, size);
>           return;
> @@ -581,7 +615,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>        * First, check for special operations in the 2K region
>        */
>       if (offset & TM_SPECIAL_OP) {
> -        xto = xive_tm_find_op(offset, size, false);
> +        xto = xive_tm_find_op(tctx->xptr, offset, size, false);
>           if (!xto) {
>               qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid read access to TIMA"
>                             "@%"HWADDR_PRIx"\n", offset);
> @@ -594,7 +628,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>       /*
>        * Then, for special operations in the region below 2K.
>        */
> -    xto = xive_tm_find_op(offset, size, false);
> +    xto = xive_tm_find_op(tctx->xptr, offset, size, false);
>       if (xto) {
>           ret = xto->read_handler(xptr, tctx, offset, size);
>           goto out;

Re: [PATCH 0/2] pnv/xive2: Fix TIMA special ops detection

[Cédric Le Goater]

On 6/21/23 18:03, Frederic Barrat wrote:
> Fix the TIMA special ops detection regression, as spotted by Coverity.
> 
> Tested by running a pseries guest on top of a powernv9 and powernv10 host.

FYI,

It is possible to force Gen1 on XIVE2 also. It you set the "capabilities"
property on the command line :

   -global driver=pnv-xive2,property=capabilities,value=0x1000120076f000FC

OPAL should self adapt and QEMU will probably crash ...

C.

> 
> Frederic Barrat (2):
>    pnv/xive2: Add a get_config() method on the presenter class
>    pnv/xive2: Check TIMA special ops against a dedicated array for P10
> 
>   hw/intc/pnv_xive.c    | 11 +++++++++
>   hw/intc/pnv_xive2.c   | 44 +++++++++------------------------
>   hw/intc/xive.c        | 57 +++++++++++++++++++++++++++++++++++++------
>   include/hw/ppc/xive.h |  3 +++
>   4 files changed, 75 insertions(+), 40 deletions(-)
> 

Re: [PATCH 0/2] pnv/xive2: Fix TIMA special ops detection

[Frederic Barrat]

On 21/06/2023 19:23, Cédric Le Goater wrote:
> FYI,
> 
> It is possible to force Gen1 on XIVE2 also. It you set the "capabilities"
> property on the command line :
> 
>    -global driver=pnv-xive2,property=capabilities,value=0x1000120076f000FC


Thanks, I hadn't noticed. And "config" too! Good to know...

   Fred