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

[Peter Maydell]

On Mon, 26 Sept 2022 at 16:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> 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?

That's just an assert by another name, and isn't any better.

> 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".

You can do that, or just make the whole memory transaction
return 0, or return a suitable memtx error.

-- PMM