Re: [PATCH v2 05/11] hw/intc/gic: use MxTxAttrs to divine accessing CPU

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 26 Sept 2022 at 16:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > On Mon, 26 Sept 2022 at 14:39, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >> -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_type == MEMTXATTRS_CPU);
>> >> +    g_assert(attrs.requester_id < s->num_cpu);
>> >
>> > Would it be a QEMU bug, or a guest code bug ? If it's possible
>> > for the guest to mis-program a DMA controller to do a read that
>> > goes through this function, we shouldn't assert. (Whether that
>> > can happen will depend on how the board/SoC code puts together
>> > the MemoryRegion hierarchy, I think.)
>>
>> Most likely a QEMU bug - how would a DMA master even access the GIC?
>
> If it's mapped into the system address space, the same way
> as it does any memory access. For instance on the virt board
> we just map the distributor MemoryRegion straight into the
> system address space, and any DMA master can talk to it.
> This is of course not how the hardware really works (where
> the GIC is part of the CPU itself), but, as noted in previous
> threads, up-ending the MemoryRegion handling in order to be
> able to put the GIC only in the address space(s) that the CPU
> sees would be a lot of work, which is why we didn't try to
> solve the "how do you figure out which CPU is writing to the
> GIC" problem that way.

So hw_error?

I don't think there is a way we can safely continue unless we just want
to fallback to "it was vCPU 0 what did it".

>
> thanks
> -- PMM


-- 
Alex Bennée