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

[Peter Maydell <peter.maydell@linaro.org>]

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.

thanks
-- PMM