On Mon, 26 Sept 2022 at 16:42, Alex Bennée <[email protected]> wrote: > > > Peter Maydell <[email protected]> writes: > > > On Mon, 26 Sept 2022 at 16:08, Alex Bennée <[email protected]> wrote: > >> Peter Maydell <[email protected]> writes: > >> > On Mon, 26 Sept 2022 at 14:39, Alex Bennée <[email protected]> > >> > 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
