Arm gic mmio mapping #10690
Merged
Arm gic mmio mapping #10690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tianocore-assign-reviewers
bot
requested review from
kraxel,
leiflindholm and
samimujawar
ardbiesheuvel
force-pushed
the
arm-gic-mmio-mapping
branch
from
ee22f6e to
bd5f1ce
Compare
ardbiesheuvel
force-pushed
the
arm-gic-mmio-mapping
branch
from
ea7b1c3 to
fc457e0
Compare
Failure to install the CPU arch protocol is a fatal error, so treat it as such, rather than ignore it, even though we won't get very far if this driver fails to dispatch - at least, we will get an error in a DEBUG build rather than a mysterious failure due to unsatisfied DEPEXes. Failure to install the idle loop event handler is not a fatal error, and it should not cause the driver to exit with an error, as this will unload the driver and keep the installed CPU arch protocol pointer dangling. So keep the ASSERT() on the return value, but return EFI_SUCCESS once we're past the point where the CPU arch protocol has been installed. Since the protocol is never uninstalled, make the CPU handle function local, as there is no point in keeping its value around. Signed-off-by: Ard Biesheuvel <[email protected]>
Use static linkage for variables and routines that are not referenced from other objects. This is generally preferred, because it gives the compiler more freedom for optimization. Signed-off-by: Ard Biesheuvel <[email protected]>
There is some fossilized code in the CpuDxe driver startup code that permits a vector table to be inherited from the PEI stage, but this code is essentially dead on ARM platforms, given that the VectorInfo argument passed to InitializeCpuExceptionHandlers() is ignored, and no code appears to exist that would result in the gEfiVectorHandoffTableGuid configuration table ever being populated. Also, due to prior refactoring, the code that disables and re-enables IRQs and FIQs is completely pointless, and can simply be removed. That, in turn, allows the CPU arch protocol parameter to be dropped from the prototype of InitializeExceptions(). Signed-off-by: Ard Biesheuvel <[email protected]>
Currently, ArmPkg's CpuDxe DEPEXes on the hardware interrupt protocol, to ensure that it is not dispatched before the GIC driver. This way, the CpuDxe driver is guaranteed not to enable interrupts on the CPU side before the GIC driver has had the opportunity to configure the interrupts on the distribution side. However, this prevents the GIC driver from using any of the CPU arch protocol interfaces, such as mapping memory, which it may need to do on platforms where the GIC MMIO regions are not mapped yet when the driver is started. So instead, use a protocol notification on the hardware interrupt protocol, which is installed by the GIC driver (as well as other existing interrupt controller drivers for platforms that do not implement a GIC) after it starts up and deasserts and disables all incoming interrupts. Manipulate the interrupt state as usual only after this notification has been received. Before that, keep track of the caller's intent regarding the interrupt enabled state in a shadow variable, but do not actually enable interrupt delivery to the CPU just yet. Signed-off-by: Ard Biesheuvel <[email protected]>
Instead of relying on a protocol notification event to register the core IRQ interrupt handler with CPU arch protocol once it becomes available, use a DEPEX to ensure that the GIC driver is not dispatched at all until the CPU arch protocol has turned up. This will allow the GIC driver to use other CPU arch protocol methods, such as the ones needed to map the GIC MMIO regions at driver startup. Signed-off-by: Ard Biesheuvel <[email protected]>
The GIC DXE driver only runs on the boot CPU, and so there is really no point in iterating over all the redistributor frames every time an interrupt is enabled, disabled or its state tested. Instead, do this only at load time. Signed-off-by: Ard Biesheuvel <[email protected]>
The GIC distributor and redistributor addresses that are passed into the interrupt enable and disable routines are always the same, so just use the global variables directly. Signed-off-by: Ard Biesheuvel <[email protected]>
The GIC driver itself has intimate knowledge of the hardware, and so it is the best suited to create the mappings of the MMIO control regions, in case they have not been mapped yet by the platform code. So call in the the CPU arch protocol to map the CPU interface, distributor and redistributor regions as they are discovered by the GIC driver startup code. Note that creating these mappings has no effect if the regions in question have already been mapped with the correct attributes. Signed-off-by: Ard Biesheuvel <[email protected]>
Currently, the ArmVirtQemu startup code maps a 128 MiB of MMIO space, as it knows that the UART, GIC and RTC live there. Now that the GIC driver maps its MMIO registers itself, there is no need for this region to cover its MMIO space. And there are other regions here that don't need to be mapped by default: the only ones that need to be mapped are the UARTS, the RTC, the fw_cfg MMIO interface and the virtio-mmio regions, all of which live in a 32 MiB window starting at address 0x900_0000. Signed-off-by: Ard Biesheuvel <[email protected]>
ardbiesheuvel
force-pushed
the
arm-gic-mmio-mapping
branch
from
fc457e0 to
d73e985
Compare
leiflindholm
approved these changes
Jan 30, 2025
|
Last patch in this series breaks my guests. |
|
If you send a revert pr I'll merge it right away - it's not that important |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the CPU and GIC DXE drivers so that the latter can invoke the CPU arch protocol exposed by the former to map the MMIO control registers explicitly.