[RFC,0/4] hw/intc: enable GICv4 memory layout for GICv3 driver

Message

Leif Lindholm Jan. 24, 2021, 2:53 a.m. UTC

GICv4 sets aside 256K per redistributor configuration block, whereas GICv3
only uses 128K. However, some codebases (like TF-A, EDK2) will happily use
the GICv3 functionality only.

This set aims at enabling these codebases to run, without actually enabling
full support for GICv4. 

This creates a ... problematic ... system, which will misbehave if you try
to use the virtual LPIs. But it does help with letting me use QEMU for
modelling a platform containing a GICv4, and share firmware images with
other prototyping platforms.

Leif Lindholm (4):
  hw/intc: don't bail out gicv3 model init for revision 4
  hw/intc: add helper function to determine gicv3 redistributor size
  hw/intc: set GICD_TYPER.DVIS for GICv4
  hw/intc: make gicv3_idreg() distinguish between gicv3/gicv4

 hw/intc/arm_gicv3_common.c         |  4 ++--
 hw/intc/arm_gicv3_dist.c           |  5 ++++-
 hw/intc/arm_gicv3_redist.c         | 15 ++++++++++-----
 hw/intc/gicv3_internal.h           | 12 ++++++++++--
 include/hw/intc/arm_gicv3_common.h |  3 +++
 5 files changed, 29 insertions(+), 10 deletions(-)

Comments

[e-mail] Jan. 24, 2021, 3 a.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [e-mail]
Subject: [RFC PATCH 0/4] hw/intc: enable GICv4 memory layout for GICv3 driver

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
22ce3eb hw/intc: make gicv3_idreg() distinguish between gicv3/gicv4
4d2400c hw/intc: set GICD_TYPER.DVIS for GICv4
bab9208 hw/intc: add helper function to determine gicv3 redistributor size
71fa48f hw/intc: don't bail out gicv3 model init for revision 4

=== OUTPUT BEGIN ===
1/4 Checking commit 71fa48fa1624 (hw/intc: don't bail out gicv3 model init for revision 4)
2/4 Checking commit bab920855fbe (hw/intc: add helper function to determine gicv3 redistributor size)
3/4 Checking commit 4d2400cda07b (hw/intc: set GICD_TYPER.DVIS for GICv4)
ERROR: superfluous trailing semicolon
#25: FILE: hw/intc/arm_gicv3_dist.c:391:
+            *data |= (1 << 18);;

total: 1 errors, 0 warnings, 9 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/4 Checking commit 22ce3eb6f90a (hw/intc: make gicv3_idreg() distinguish between gicv3/gicv4)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Peter Maydell Feb. 2, 2021, 10:39 a.m. UTC | #2

On Sun, 24 Jan 2021 at 02:53, Leif Lindholm <[e-mail]> wrote:
>
> GICv4 sets aside 256K per redistributor configuration block, whereas GICv3
> only uses 128K. However, some codebases (like TF-A, EDK2) will happily use
> the GICv3 functionality only.
>
> This set aims at enabling these codebases to run, without actually enabling
> full support for GICv4.
>
> This creates a ... problematic ... system, which will misbehave if you try
> to use the virtual LPIs. But it does help with letting me use QEMU for
> modelling a platform containing a GICv4, and share firmware images with
> other prototyping platforms.

So, what's your aim for this series? I think we could reasonably
take patches 2 and 4 upstream (they are changes we'll want for eventual
v4 emulation support), but I don't really want 1 and 3.
That would reduce the delta you're carrying locally, at least.

I suppose we should look at what changes QEMU needs for KVM in-kernel GICv4
support at some point...

thanks
-- PMM

Leif Lindholm Feb. 3, 2021, 12:26 p.m. UTC | #3

On Tue, Feb 02, 2021 at 10:39:22 +0000, Peter Maydell wrote:
> On Sun, 24 Jan 2021 at 02:53, Leif Lindholm <[e-mail]> wrote:
> >
> > GICv4 sets aside 256K per redistributor configuration block, whereas GICv3
> > only uses 128K. However, some codebases (like TF-A, EDK2) will happily use
> > the GICv3 functionality only.
> >
> > This set aims at enabling these codebases to run, without actually enabling
> > full support for GICv4.
> >
> > This creates a ... problematic ... system, which will misbehave if you try
> > to use the virtual LPIs. But it does help with letting me use QEMU for
> > modelling a platform containing a GICv4, and share firmware images with
> > other prototyping platforms.
> 
> So, what's your aim for this series? I think we could reasonably
> take patches 2 and 4 upstream (they are changes we'll want for eventual
> v4 emulation support), but I don't really want 1 and 3.
> That would reduce the delta you're carrying locally, at least.

That would be much appreciated.

In a way, I wanted to test the waters a bit with regards to whether
gicv4 emulation could be introduced gradually, and whether doing so by
extending the existing gicv3 driver was the way to go.

Anyway, for now, I'll address your comments and send out a 2-part v2,
containing 2,4/4.

Best Regards,

Leif

> I suppose we should look at what changes QEMU needs for KVM in-kernel GICv4
> support at some point...
> 
> thanks
> -- PMM