On Fri, May 01, 2015 at 06:50:26PM +0100, Peter Maydell wrote: > This patch series adds support for GICv1 and GICv2 security > extensions, as well as support for GIC interrupt grouping on GICv2.
A question. Once we enable the the security extensions on the GICs, do you have any suggestions on howto best handle direct boots into NS EL2/1 (Linux)? The GIC resets to all interrupts configured for Group0 and Linux running in NS mode cannot change that so we need some kind of boot-loader code or magic to do what firmware would have been expected to do at boot time (switch some irqs to NS). Cheers, Edgar > > This is based on the work originally by Fabian and then by Greg. > I've gone through and dealt with all the issues I raised in code > review, and a few others I noticed as I was working on it. > The general structure of the changes is still the same, though > I've reordered one or two of the patches; I've touched most of > the lines of code in the series, though, as well as deleting > quite a few (the patches now add ~375 lines of code rather than > over 475). > > I think these patches are in a suitable state to apply (and > they have no dependencies that aren't in master), assuming no > further issues found in review. > > With this patchset the security extensions are still disabled > on all boards, so the actual functional change is that GICv2 > now correctly implements interrupt grouping. This is enabled > always for GICv2, because the programming model is fully backwards > compatible with treating the GIC like one which doesn't support > groups (which is what Linux does). > > The next part of this work is going to be actually enabling > the security extensions. Here's a sketch of my plan for that: > * the a15mpcore and a9mpcore wrapper objects will default to > enabling the security extensions in the GIC they create > (unless the GIC is the KVM one). They also provide a > QOM property to override this > * for the set of legacy boards which are currently disabling > has_el3 on their CPUs, we also have them disable TZ in the GIC > (a non-TZ CPU and a TZ GIC is a bad combo because the CPU > has no way to put the interrupts into Group1 where it can > use them, so the whole system is busted) > * the virt board creates its GIC directly, so it should also > set the has-security-extensions property as needed > * if boot.c is starting the CPUs directly in NonSecure > mode (because we're booting a kernel directly rather than > starting firmware, and arm_boot_info::secure_boot is false) > then it must also manually configure the GIC distributor > to put all interrupts into Group1. This is boot.c having > to do a firmware configuration job since it's effectively > acting as lightweight builtin firmware. > > I think we could reasonably review and commit this patchseries > without waiting for that bit of board-wiring work; let me know > if you disagree. > > > Major changes since v3: > * renamed property to 'has-security-extensions', to be a bit > more in line with the CPU's 'has_el3'. I'm not wedded to this > name so if anybody wants to suggest something better (or > tell me our convention for prop names is underscores!) feel free > * error on realize if security extensions turned on for a GIC > which doesn't support them > * new patch: switch to read/write_with_attrs MMIO callbacks so > we can get at the Secure/NonSecure tx attribute > * make the GIC_*_GROUP macros work like the others, with a simple > SET/CLEAR/TEST semantic > * new patch: save and restore GICD_IGROUPRn state when using KVM > now we have the state struct fields to keep it in [the kernel > doesn't implement grouping, but if it ever does we will be ready] > * rather than having a 2-element array for storing the S and NS > banked versions of GICD_CTLR and GICC_CTLR, just store the S > version, since in both cases the NS view is just an alias of > a subset of bits from the S register. This allows us to nicely > simplify a lot of the logic that deals with these registers. > * fixed bug in handling of GICC_BPR for GICv2-without-TZ > * added missing masks in gic_set_priority_mask() and gic_set_priority() > * make AckCtl operate on GICv2-without-TZ > * handle an UNPREDICTABLE case (Secure EOI of a Group1 irq > with AckCtl == 0) in a way more convenient for the implementation > * reuse gic_get_current_pending_irq() in implementation of IAR writes, > rather than reimplementing equivalent logic > * new patch: support grouping in a single gic_update function (rather > than having split update functions for the two cases) > * new patch: wire FIQ up on highbank/midway; this means we're now > consistent in having FIQ wired up on all our boards with GICv2 > * lots of minor formatting tweaks, etc; see individual commit messages > > > Fabian Aggeler (12): > hw/intc/arm_gic: Create outbound FIQ lines > hw/intc/arm_gic: Add Security Extensions property > hw/intc/arm_gic: Add Interrupt Group Registers > hw/intc/arm_gic: Make ICDDCR/GICD_CTLR banked > hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked > hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked > hw/intc/arm_gic: Implement Non-secure view of RPR > hw/intc/arm_gic: Restrict priority view > hw/intc/arm_gic: Handle grouping for GICC_HPPIR > hw/intc/arm_gic: Change behavior of EOIR writes > hw/intc/arm_gic: Change behavior of IAR writes > hw/arm/vexpress.c: Wire FIQ between CPU <> GIC > > Greg Bellows (1): > hw/arm/virt.c: Wire FIQ between CPU <> GIC > > Peter Maydell (4): > hw/intc/arm_gic: Switch to read/write callbacks with tx attributes > hw/intc/arm_gic_kvm.c: Save and restore GICD_IGROUPRn state > hw/intc/arm_gic: Add grouping support to gic_update() > hw/arm/highbank.c: Wire FIQ between CPU <> GIC > > hw/arm/highbank.c | 3 + > hw/arm/vexpress.c | 2 + > hw/arm/virt.c | 2 + > hw/intc/arm_gic.c | 469 > ++++++++++++++++++++++++++++++++------- > hw/intc/arm_gic_common.c | 22 +- > hw/intc/arm_gic_kvm.c | 51 +++-- > hw/intc/armv7m_nvic.c | 8 +- > hw/intc/gic_internal.h | 29 ++- > include/hw/intc/arm_gic_common.h | 24 +- > 9 files changed, 492 insertions(+), 118 deletions(-) > > -- > 1.9.1 >