Re: [Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter

2018-12-11 Thread Cédric Le Goater
On 12/11/18 2:37 AM, David Gibson wrote:
> On Mon, Dec 10, 2018 at 08:15:40AM +0100, Cédric Le Goater wrote:
>> On 12/10/18 5:27 AM, David Gibson wrote:
>>> On Sun, Dec 09, 2018 at 08:45:54PM +0100, Cédric Le Goater wrote:
 The last sub-engine of the XIVE architecture is the Interrupt
 Virtualization Presentation Engine (IVPE). On HW, the IVRE and the
 IVPE share elements, the Power Bus interface (CQ), the routing table
 descriptors, and they can be combined in the same HW logic. We do the
 same in QEMU and combine both engines in the XiveRouter for
 simplicity.

 When the IVRE has completed its job of matching an event source with a
 Notification Virtual Target (NVT) to notify, it forwards the event
 notification to the IVPE sub-engine. The IVPE scans the thread
 interrupt contexts of the Notification Virtual Targets (NVT)
 dispatched on the HW processor threads and if a match is found, it
 signals the thread. If not, the IVPE escalates the notification to
 some other targets and records the notification in a backlog queue.

 The IVPE maintains the thread interrupt context state for each of its
 NVTs not dispatched on HW processor threads in the Notification
 Virtual Target table (NVTT).

 The model currently only supports single NVT notifications.

 Signed-off-by: Cédric Le Goater 
>>>
>>> Applied.
>>>
>>> I think the tctx_word2() should have the byteswap, rather than having
>>> it in the callers, but that can be fixed later.
>>
>> I thought it was better to explicitly show in the code where the 
>> byteswaps were needed. Anyway, this is very localized, so, yes, 
>> we can change it later on.
> 
> To clarify my thinking here; the important thing is not knowing where
> the byteswaps are, but being able to tell as easily as possible what
> endianness a given piece of data is right now.
> 
> The convention I'm aiming for here - which is one I try to use most
> places is that structures - at least structures which map to specific
> in-memory things - are in the required endianness for that stuff in
> memory.  However bare integers - uint32_t or uint64_t or whatever -
> are, well, numbers, in native endian.
> 

ok. Adding that to the TODO list.

C.



Re: [Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter

2018-12-10 Thread David Gibson
On Mon, Dec 10, 2018 at 08:15:40AM +0100, Cédric Le Goater wrote:
> On 12/10/18 5:27 AM, David Gibson wrote:
> > On Sun, Dec 09, 2018 at 08:45:54PM +0100, Cédric Le Goater wrote:
> >> The last sub-engine of the XIVE architecture is the Interrupt
> >> Virtualization Presentation Engine (IVPE). On HW, the IVRE and the
> >> IVPE share elements, the Power Bus interface (CQ), the routing table
> >> descriptors, and they can be combined in the same HW logic. We do the
> >> same in QEMU and combine both engines in the XiveRouter for
> >> simplicity.
> >>
> >> When the IVRE has completed its job of matching an event source with a
> >> Notification Virtual Target (NVT) to notify, it forwards the event
> >> notification to the IVPE sub-engine. The IVPE scans the thread
> >> interrupt contexts of the Notification Virtual Targets (NVT)
> >> dispatched on the HW processor threads and if a match is found, it
> >> signals the thread. If not, the IVPE escalates the notification to
> >> some other targets and records the notification in a backlog queue.
> >>
> >> The IVPE maintains the thread interrupt context state for each of its
> >> NVTs not dispatched on HW processor threads in the Notification
> >> Virtual Target table (NVTT).
> >>
> >> The model currently only supports single NVT notifications.
> >>
> >> Signed-off-by: Cédric Le Goater 
> > 
> > Applied.
> > 
> > I think the tctx_word2() should have the byteswap, rather than having
> > it in the callers, but that can be fixed later.
> 
> I thought it was better to explicitly show in the code where the 
> byteswaps were needed. Anyway, this is very localized, so, yes, 
> we can change it later on.

To clarify my thinking here; the important thing is not knowing where
the byteswaps are, but being able to tell as easily as possible what
endianness a given piece of data is right now.

The convention I'm aiming for here - which is one I try to use most
places is that structures - at least structures which map to specific
in-memory things - are in the required endianness for that stuff in
memory.  However bare integers - uint32_t or uint64_t or whatever -
are, well, numbers, in native endian.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter

2018-12-09 Thread Cédric Le Goater
On 12/10/18 5:27 AM, David Gibson wrote:
> On Sun, Dec 09, 2018 at 08:45:54PM +0100, Cédric Le Goater wrote:
>> The last sub-engine of the XIVE architecture is the Interrupt
>> Virtualization Presentation Engine (IVPE). On HW, the IVRE and the
>> IVPE share elements, the Power Bus interface (CQ), the routing table
>> descriptors, and they can be combined in the same HW logic. We do the
>> same in QEMU and combine both engines in the XiveRouter for
>> simplicity.
>>
>> When the IVRE has completed its job of matching an event source with a
>> Notification Virtual Target (NVT) to notify, it forwards the event
>> notification to the IVPE sub-engine. The IVPE scans the thread
>> interrupt contexts of the Notification Virtual Targets (NVT)
>> dispatched on the HW processor threads and if a match is found, it
>> signals the thread. If not, the IVPE escalates the notification to
>> some other targets and records the notification in a backlog queue.
>>
>> The IVPE maintains the thread interrupt context state for each of its
>> NVTs not dispatched on HW processor threads in the Notification
>> Virtual Target table (NVTT).
>>
>> The model currently only supports single NVT notifications.
>>
>> Signed-off-by: Cédric Le Goater 
> 
> Applied.
> 
> I think the tctx_word2() should have the byteswap, rather than having
> it in the callers, but that can be fixed later.

I thought it was better to explicitly show in the code where the 
byteswaps were needed. Anyway, this is very localized, so, yes, 
we can change it later on.

C.

> 
>> ---
>>
>>  Changes since v6 :
>>
>>  - removed HW CAM line setting and use as it is only useful for PowerNV
>>  - made use of xive_tctx_word2() helper
>>  - made use of GETFIELD_BE32() to compare CAM lines
>>  - fixed initialization of XiveTCTXMatch
>>
>>  include/hw/ppc/xive.h  |  14 +++
>>  include/hw/ppc/xive_regs.h |  24 +
>>  hw/intc/xive.c | 185 +
>>  3 files changed, 223 insertions(+)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 1e823a4c64e9..19309d1d65d1 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -325,6 +325,10 @@ typedef struct XiveRouterClass {
>> XiveEND *end);
>>  int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>>   XiveEND *end, uint8_t word_number);
>> +int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> +   XiveNVT *nvt);
>> +int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> + XiveNVT *nvt, uint8_t word_number);
>>  } XiveRouterClass;
>>  
>>  void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
>> @@ -335,6 +339,11 @@ int xive_router_get_end(XiveRouter *xrtr, uint8_t 
>> end_blk, uint32_t end_idx,
>>  XiveEND *end);
>>  int xive_router_write_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t 
>> end_idx,
>>XiveEND *end, uint8_t word_number);
>> +int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> +XiveNVT *nvt);
>> +int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t 
>> nvt_idx,
>> +  XiveNVT *nvt, uint8_t word_number);
>> +
>>  
>>  /*
>>   * XIVE END ESBs
>> @@ -411,4 +420,9 @@ extern const MemoryRegionOps xive_tm_ops;
>>  
>>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>>  
>> +static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> +{
>> +return (nvt_blk << 19) | nvt_idx;
>> +}
>> +
>>  #endif /* PPC_XIVE_H */
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index ede3d04c5eda..85557e730cd8 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -186,4 +186,28 @@ typedef struct XiveEND {
>>  #define GETFIELD_BE32(m, v)   GETFIELD(m, be32_to_cpu(v))
>>  #define SETFIELD_BE32(m, v, val)  cpu_to_be32(SETFIELD(m, be32_to_cpu(v), 
>> val))
>>  
>> +/* Notification Virtual Target (NVT) */
>> +typedef struct XiveNVT {
>> +uint32_tw0;
>> +#define NVT_W0_VALID PPC_BIT32(0)
>> +uint32_tw1;
>> +uint32_tw2;
>> +uint32_tw3;
>> +uint32_tw4;
>> +uint32_tw5;
>> +uint32_tw6;
>> +uint32_tw7;
>> +uint32_tw8;
>> +#define NVT_W8_GRP_VALID PPC_BIT32(0)
>> +uint32_tw9;
>> +uint32_twa;
>> +uint32_twb;
>> +uint32_twc;
>> +uint32_twd;
>> +uint32_twe;
>> +uint32_twf;
>> +} XiveNVT;
>> +
>> +#define xive_nvt_is_valid(nvt)(be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
>> +
>>  #endif /* PPC_XIVE_REGS_H */
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 2615d16b7437..3eecffe99b3a 100644
>> --- 

Re: [Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter

2018-12-09 Thread David Gibson
On Sun, Dec 09, 2018 at 08:45:54PM +0100, Cédric Le Goater wrote:
> The last sub-engine of the XIVE architecture is the Interrupt
> Virtualization Presentation Engine (IVPE). On HW, the IVRE and the
> IVPE share elements, the Power Bus interface (CQ), the routing table
> descriptors, and they can be combined in the same HW logic. We do the
> same in QEMU and combine both engines in the XiveRouter for
> simplicity.
> 
> When the IVRE has completed its job of matching an event source with a
> Notification Virtual Target (NVT) to notify, it forwards the event
> notification to the IVPE sub-engine. The IVPE scans the thread
> interrupt contexts of the Notification Virtual Targets (NVT)
> dispatched on the HW processor threads and if a match is found, it
> signals the thread. If not, the IVPE escalates the notification to
> some other targets and records the notification in a backlog queue.
> 
> The IVPE maintains the thread interrupt context state for each of its
> NVTs not dispatched on HW processor threads in the Notification
> Virtual Target table (NVTT).
> 
> The model currently only supports single NVT notifications.
> 
> Signed-off-by: Cédric Le Goater 

Applied.

I think the tctx_word2() should have the byteswap, rather than having
it in the callers, but that can be fixed later.

> ---
> 
>  Changes since v6 :
> 
>  - removed HW CAM line setting and use as it is only useful for PowerNV
>  - made use of xive_tctx_word2() helper
>  - made use of GETFIELD_BE32() to compare CAM lines
>  - fixed initialization of XiveTCTXMatch
> 
>  include/hw/ppc/xive.h  |  14 +++
>  include/hw/ppc/xive_regs.h |  24 +
>  hw/intc/xive.c | 185 +
>  3 files changed, 223 insertions(+)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 1e823a4c64e9..19309d1d65d1 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -325,6 +325,10 @@ typedef struct XiveRouterClass {
> XiveEND *end);
>  int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>   XiveEND *end, uint8_t word_number);
> +int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> +   XiveNVT *nvt);
> +int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> + XiveNVT *nvt, uint8_t word_number);
>  } XiveRouterClass;
>  
>  void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
> @@ -335,6 +339,11 @@ int xive_router_get_end(XiveRouter *xrtr, uint8_t 
> end_blk, uint32_t end_idx,
>  XiveEND *end);
>  int xive_router_write_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t 
> end_idx,
>XiveEND *end, uint8_t word_number);
> +int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> +XiveNVT *nvt);
> +int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t 
> nvt_idx,
> +  XiveNVT *nvt, uint8_t word_number);
> +
>  
>  /*
>   * XIVE END ESBs
> @@ -411,4 +420,9 @@ extern const MemoryRegionOps xive_tm_ops;
>  
>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>  
> +static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> +{
> +return (nvt_blk << 19) | nvt_idx;
> +}
> +
>  #endif /* PPC_XIVE_H */
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index ede3d04c5eda..85557e730cd8 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -186,4 +186,28 @@ typedef struct XiveEND {
>  #define GETFIELD_BE32(m, v)   GETFIELD(m, be32_to_cpu(v))
>  #define SETFIELD_BE32(m, v, val)  cpu_to_be32(SETFIELD(m, be32_to_cpu(v), 
> val))
>  
> +/* Notification Virtual Target (NVT) */
> +typedef struct XiveNVT {
> +uint32_tw0;
> +#define NVT_W0_VALID PPC_BIT32(0)
> +uint32_tw1;
> +uint32_tw2;
> +uint32_tw3;
> +uint32_tw4;
> +uint32_tw5;
> +uint32_tw6;
> +uint32_tw7;
> +uint32_tw8;
> +#define NVT_W8_GRP_VALID PPC_BIT32(0)
> +uint32_tw9;
> +uint32_twa;
> +uint32_twb;
> +uint32_twc;
> +uint32_twd;
> +uint32_twe;
> +uint32_twf;
> +} XiveNVT;
> +
> +#define xive_nvt_is_valid(nvt)(be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
> +
>  #endif /* PPC_XIVE_REGS_H */
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 2615d16b7437..3eecffe99b3a 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -983,6 +983,183 @@ int xive_router_write_end(XiveRouter *xrtr, uint8_t 
> end_blk, uint32_t end_idx,
> return xrc->write_end(xrtr, end_blk, end_idx, end, word_number);
>  }
>  
> +int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> +

[Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter

2018-12-09 Thread Cédric Le Goater
The last sub-engine of the XIVE architecture is the Interrupt
Virtualization Presentation Engine (IVPE). On HW, the IVRE and the
IVPE share elements, the Power Bus interface (CQ), the routing table
descriptors, and they can be combined in the same HW logic. We do the
same in QEMU and combine both engines in the XiveRouter for
simplicity.

When the IVRE has completed its job of matching an event source with a
Notification Virtual Target (NVT) to notify, it forwards the event
notification to the IVPE sub-engine. The IVPE scans the thread
interrupt contexts of the Notification Virtual Targets (NVT)
dispatched on the HW processor threads and if a match is found, it
signals the thread. If not, the IVPE escalates the notification to
some other targets and records the notification in a backlog queue.

The IVPE maintains the thread interrupt context state for each of its
NVTs not dispatched on HW processor threads in the Notification
Virtual Target table (NVTT).

The model currently only supports single NVT notifications.

Signed-off-by: Cédric Le Goater 
---

 Changes since v6 :

 - removed HW CAM line setting and use as it is only useful for PowerNV
 - made use of xive_tctx_word2() helper
 - made use of GETFIELD_BE32() to compare CAM lines
 - fixed initialization of XiveTCTXMatch

 include/hw/ppc/xive.h  |  14 +++
 include/hw/ppc/xive_regs.h |  24 +
 hw/intc/xive.c | 185 +
 3 files changed, 223 insertions(+)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 1e823a4c64e9..19309d1d65d1 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -325,6 +325,10 @@ typedef struct XiveRouterClass {
XiveEND *end);
 int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
  XiveEND *end, uint8_t word_number);
+int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+   XiveNVT *nvt);
+int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+ XiveNVT *nvt, uint8_t word_number);
 } XiveRouterClass;
 
 void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
@@ -335,6 +339,11 @@ int xive_router_get_end(XiveRouter *xrtr, uint8_t end_blk, 
uint32_t end_idx,
 XiveEND *end);
 int xive_router_write_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
   XiveEND *end, uint8_t word_number);
+int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+XiveNVT *nvt);
+int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+  XiveNVT *nvt, uint8_t word_number);
+
 
 /*
  * XIVE END ESBs
@@ -411,4 +420,9 @@ extern const MemoryRegionOps xive_tm_ops;
 
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
 
+static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
+{
+return (nvt_blk << 19) | nvt_idx;
+}
+
 #endif /* PPC_XIVE_H */
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index ede3d04c5eda..85557e730cd8 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -186,4 +186,28 @@ typedef struct XiveEND {
 #define GETFIELD_BE32(m, v)   GETFIELD(m, be32_to_cpu(v))
 #define SETFIELD_BE32(m, v, val)  cpu_to_be32(SETFIELD(m, be32_to_cpu(v), val))
 
+/* Notification Virtual Target (NVT) */
+typedef struct XiveNVT {
+uint32_tw0;
+#define NVT_W0_VALID PPC_BIT32(0)
+uint32_tw1;
+uint32_tw2;
+uint32_tw3;
+uint32_tw4;
+uint32_tw5;
+uint32_tw6;
+uint32_tw7;
+uint32_tw8;
+#define NVT_W8_GRP_VALID PPC_BIT32(0)
+uint32_tw9;
+uint32_twa;
+uint32_twb;
+uint32_twc;
+uint32_twd;
+uint32_twe;
+uint32_twf;
+} XiveNVT;
+
+#define xive_nvt_is_valid(nvt)(be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
+
 #endif /* PPC_XIVE_REGS_H */
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 2615d16b7437..3eecffe99b3a 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -983,6 +983,183 @@ int xive_router_write_end(XiveRouter *xrtr, uint8_t 
end_blk, uint32_t end_idx,
return xrc->write_end(xrtr, end_blk, end_idx, end, word_number);
 }
 
+int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+XiveNVT *nvt)
+{
+   XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
+
+   return xrc->get_nvt(xrtr, nvt_blk, nvt_idx, nvt);
+}
+
+int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
+XiveNVT *nvt, uint8_t word_number)
+{
+   XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
+
+   return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
+}
+
+/*
+ * The thread context