Re: [PATCH v2 00/13] arm gicv3 ITS: Various bug fixes and refactorings

[Peter Maydell]

On Tue, 18 Jan 2022 at 23:30, Andre Przywara <andre.przywara@arm.com> wrote:
> Looking at k-u-t's arm/gic.c and QEMU's hw/intc/arm_gic.c I see two
> problems here: QEMU implements word accesses as four successive calls to
> gic_dist_readb() - which is probably fine if that helps code design,
> but it won't allow it to actually spot access size issues. I just
> remember that we spent some brain cells and CPP macros on getting the
> access size right in KVM - hence those tests in kvm-unit-tests.

Thanks for looking at this. I should have read the code rather
than dashing off a reply last thing in the evening based just
on the test case output! I think I was confusing how our GICv3
emulation handles register accesses (with separate functions for
byte/halfword/word/quad accesses) with the GICv2 emulation
(which as you say calls down into the byte emulation code
wherever it can).

> But more importantly it looks like GICD_IIDR is actually not
> implemented: There is a dubious "if (offset < 0x08) return 0;" line,
> but IIDR (offset 0x8) would actually fall through, and hit the bad_reg
> label, which would return 0 (and cause the message, if enabled).

Mmm. I actually have a patch in target-arm.next from Petr Pavlu
which implements GICC_IIDR, but we do indeed not implement the
distributor equivalent.

> If that helps: from a GIC MMIO perspective 8-bit accesses are actually
> the exception rather than the norm (ARM IHI 0048B.b 4.1.4 GIC register
> access).

Yes. We got this right in the GICv3 emulation design, where almost
all the logic is in the 32-bit accessor functions and the 8/16-bit
functions deal only with the very few registers that have to
permit non-word accesses. The GICv2 code is a lot older (and to
be fair to it, started out as 11MPcore interrupt controller
emulation, and I bet the docs of that were not very specific about
what registers could or could not be accessed byte at a time).
Unless we want to rewrite all that logic in the GICv2 emulation
(which I at least do not :-)) I think we'll have to live with
the warnings about bad-offsets reporting for each byte rather
than just once for the word access.

-- PMM