Re: [RFC PATCH 3/4] hw/intc/gic: use MxTxAttrs to divine accessing CPU

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 15/9/22 10:16, Richard Henderson wrote:
>> On 9/14/22 17:09, Alex Bennée wrote:
>>> Now that MxTxAttrs encodes a CPU we should use that to figure it out.
>>> This solves edge cases like accessing via gdbstub or qtest.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/124
>>> ---
>>>   hw/intc/arm_gic.c | 39 ++++++++++++++++++++++-----------------
>>>   1 file changed, 22 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>>> index 492b2421ab..7feedac735 100644
>>> --- a/hw/intc/arm_gic.c
>>> +++ b/hw/intc/arm_gic.c
>>> @@ -56,17 +56,22 @@ static const uint8_t gic_id_gicv2[] = {
>>>       0x04, 0x00, 0x00, 0x00, 0x90, 0xb4, 0x2b, 0x00, 0x0d, 0xf0,
>>> 0x05, 0xb1
>>>   };
>>> -static inline int gic_get_current_cpu(GICState *s)
>>> +static inline int gic_get_current_cpu(GICState *s, MemTxAttrs attrs)
>>>   {
>>> -    if (!qtest_enabled() && s->num_cpu > 1) {
>>> -        return current_cpu->cpu_index;
>>> -    }
>>> -    return 0;
>>> +    /*
>>> +     * Something other than a CPU accessing the GIC would be a bug as
>>> +     * would a CPU index higher than the GICState expects to be
>>> +     * handling
>>> +     */
>>> +    g_assert(attrs.requester_cpu == 1);
>> Better without "== 1" -- this field ought to be boolean.
>
> Boolean so far, but this could get more types (such DMA...).
> Maybe we could already add an enum definitions, i.e.:
>
> typedef enum MemTxRequesterType {
>   MEMTXATTRS_CPU,
>   MEMTXATTRS_MSI,
> } MemTxRequesterType;
>
> and name the field MemTxAttrs::requester_type.

I pondered boolean but wasn't sure if that would blow up the size of
MemTxAttrs so went for the bitfield. However I can certainly rename to
requester_is_cpu and make a boolean assertion. I'd hold off adding an
enum until we actually have more than two requester types.

-- 
Alex Bennée