Re: [PATCH v2 4/5] pnv/xive2: Introduce macros to manipulate TIMA addresses

[Cédric Le Goater]

On 6/1/23 14:13, Frederic Barrat wrote:
> TIMA addresses are somewhat special and are split in several bit
> fields with different meanings. This patch describes it and introduce
> macros to more easily access the various fields.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>


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

Thanks,

C.


> ---
>   hw/intc/xive.c             | 14 +++++++-------
>   include/hw/ppc/xive_regs.h | 16 ++++++++++++++++
>   2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index a986b96843..ebe399bc09 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -249,7 +249,7 @@ static const uint8_t *xive_tm_views[] = {
>   static uint64_t xive_tm_mask(hwaddr offset, unsigned size, bool write)
>   {
>       uint8_t page_offset = (offset >> TM_SHIFT) & 0x3;
> -    uint8_t reg_offset = offset & 0x3F;
> +    uint8_t reg_offset = offset & TM_REG_OFFSET;
>       uint8_t reg_mask = write ? 0x1 : 0x2;
>       uint64_t mask = 0x0;
>       int i;
> @@ -266,8 +266,8 @@ static uint64_t xive_tm_mask(hwaddr offset, unsigned size, 
> bool write)
>   static void xive_tm_raw_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>                                 unsigned size)
>   {
> -    uint8_t ring_offset = offset & 0x30;
> -    uint8_t reg_offset = offset & 0x3F;
> +    uint8_t ring_offset = offset & TM_RING_OFFSET;
> +    uint8_t reg_offset = offset & TM_REG_OFFSET;
>       uint64_t mask = xive_tm_mask(offset, size, true);
>       int i;
>   
> @@ -296,8 +296,8 @@ static void xive_tm_raw_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>   
>   static uint64_t xive_tm_raw_read(XiveTCTX *tctx, hwaddr offset, unsigned size)
>   {
> -    uint8_t ring_offset = offset & 0x30;
> -    uint8_t reg_offset = offset & 0x3F;
> +    uint8_t ring_offset = offset & TM_RING_OFFSET;
> +    uint8_t reg_offset = offset & TM_REG_OFFSET;
>       uint64_t mask = xive_tm_mask(offset, size, false);
>       uint64_t ret;
>       int i;
> @@ -534,7 +534,7 @@ void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX 
> *tctx, hwaddr offset,
>       /*
>        * First, check for special operations in the 2K region
>        */
> -    if (offset & 0x800) {
> +    if (offset & TM_SPECIAL_OP) {
>           xto = xive_tm_find_op(offset, size, true);
>           if (!xto) {
>               qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA 
> "
> @@ -573,7 +573,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX 
> *tctx, hwaddr offset,
>       /*
>        * First, check for special operations in the 2K region
>        */
> -    if (offset & 0x800) {
> +    if (offset & TM_SPECIAL_OP) {
>           xto = xive_tm_find_op(offset, size, false);
>           if (!xto) {
>               qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid read access to TIMA"
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index b7fde2354e..4a3c9badd3 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -48,6 +48,22 @@
>   
>   #define TM_SHIFT                16
>   
> +/*
> + * TIMA addresses are 12-bits (4k page).
> + * The MSB indicates a special op with side effect, which can be
> + * refined with bit 10 (see below).
> + * The registers, logically grouped in 4 rings (a quad-word each), are
> + * defined on the 6 LSBs (offset below 0x40)
> + * In between, we can add a cache line index from 0...3 (ie, 0, 0x80,
> + * 0x100, 0x180) to select a specific snooper. Those 'snoop port
> + * address' bits should be dropped when processing the operations as
> + * they are all equivalent.
> + */
> +#define TM_ADDRESS_MASK         0xC3F
> +#define TM_SPECIAL_OP           0x800
> +#define TM_RING_OFFSET          0x30
> +#define TM_REG_OFFSET           0x3F
> +
>   /* TM register offsets */
>   #define TM_QW0_USER             0x000 /* All rings */
>   #define TM_QW1_OS               0x010 /* Ring 0..2 */