On 21/06/2023 09:18, Cédric Le Goater wrote:
The XIVE2 TM ops are implemented with a shortcut (See the TODO in
pnv_xive2_tm_*()). We could
1. extend xive_tctx_tm_write/read with a 'bool gen1_tima_os' parameter:
xive_tctx_tm_write(xptr, tctx, offset, value, size, gen1_tima_os);
and use the bool in xive_tm_find_op() to select a XiveTmOp array.
The new xive2_tm_operations[] would be defined as xive_tm_operations[]
but with an override of HV_PUSH_OS_CTX_OFFSET and
HV_PULL_OS_CTX_OFFSET.
2. or extend the XivePresenterClass with a get_config() handler like it
was done for Xive2RouterClass().
3. or introduce an array of XiveTmOp under XivePresenterClass defined by
the controller variant.
In any case, we need a new xive2_tm_operations[] for the XIVE2 TM register
layout. Option 1 is simpler I think.
I was also leaning on introducing a xive2_tm_operations[] array of
operations to fix it correctly.
While I agree it's the simplest, I'm not fond of (1), since we'd need to
carry that extra parameter to xive_tm_find_op(). Admittedly it's just
one extra level, but I went with something which is hopefully what you
had in mind for (2). I like that we can easily extend it in the future.
This would also "fix" the indirect ops in XIVE2, not that we care much
but it will be cleaner.
I'm not sure I see what you mean here. It cleans up nicely
pnv_xive2_tm_read/write(), but is that really what you had in mind?
Something related I notice is that when doing an indirect access to the
TIMA through the IC BAR, we call the TIMA access functions with a NULL
reference to the presenter:
static uint64_t pnv_xive2_ic_tm_indirect_read(void *opaque, hwaddr offset,
unsigned size)
{
PnvXive2 *xive = PNV_XIVE2(opaque);
...
tctx = pnv_xive2_get_indirect_tctx(xive, pir);
if (tctx) {
val = xive_tctx_tm_read(NULL, tctx, offset, size);
}
We seem to mostly ignore that first parameter in
xive_tctx_tm_read/write(). IIUC, the special ops will be checked with a
page offset matching ring 0 and won't match anything. Still, it seems a
bit dangerous and I was wondering:
1. can't we just create it from the PnvXive2 we have at hand?
2. in any case, isn't it always redundant with tctxt->presenter?
Fred