Hello team,
Currently the amount of memory allocated for GICR frames is determined by the number of supported CPUs. However, the GIC redistributor might have more frames than the number of PEs. In such a case, it is possible that the core index constructed from GICR_TYPER register points to a non-existent PE. For such a case, the GIC discovery and init sequence should move to the next redistributor frame. Today, the code asserts if there are holes in the CPU topology or if GICR_FRAMES > MAX_CPUS.
Downstream Tegra platforms provide more GICR frames than number of CPUs and require the support posted to gerrit [1]. Request the team to review and post feedback.
Thanks.
[1] topic:"gicv3-gicr-frames" (status:open OR status:merged) * Gerrit Code Review (trustedfirmware.org)https://review.trustedfirmware.org/q/topic:%22gicv3-gicr-frames%22+(status:open%20OR%20status:merged)
Hi Varun,
Out of curiosity, I'm trying to understand how far this change is generic or a specific chipset workaround? In other words, how does this deviate from the Arm architecture?... Is this a common practise to disable PEs but leave their MPIDR identification into the GIC?
The redistributor probing implemented in Hafnium is very much the same pattern as in the TF-A GIC driver. In the Hafnium case, would it be nicer to walk the list of cpus declared in the spmc manifest, and match the corresponding MPIDR from redistributor?
Regards, Olivier.
________________________________________ From: Varun Wadekar via Hafnium hafnium@lists.trustedfirmware.org Sent: 14 February 2022 19:04 To: hafnium@lists.trustedfirmware.org Cc: Bo Yan Subject: [Hafnium] GICR discovery
Hello team,
Currently the amount of memory allocated for GICR frames is determined by the number of supported CPUs. However, the GIC redistributor might have more frames than the number of PEs. In such a case, it is possible that the core index constructed from GICR_TYPER register points to a non-existent PE. For such a case, the GIC discovery and init sequence should move to the next redistributor frame. Today, the code asserts if there are holes in the CPU topology or if GICR_FRAMES > MAX_CPUS.
Downstream Tegra platforms provide more GICR frames than number of CPUs and require the support posted to gerrit [1]. Request the team to review and post feedback.
Thanks.
[1] topic:"gicv3-gicr-frames" (status:open OR status:merged) * Gerrit Code Review (trustedfirmware.org)https://review.trustedfirmware.org/q/topic:%22gicv3-gicr-frames%22+(status:open%20OR%20status:merged) -- Hafnium mailing list -- hafnium@lists.trustedfirmware.org To unsubscribe send an email to hafnium-leave@lists.trustedfirmware.org
Hi,
If we look at the GIC spec, the last GICR frame is marked with GICR_TYPER bit 4 set to '1'. The spec does not mandate that GICR frames must match the number of CPUs. Plus, there can be holes in the CPU topology due to manufacturing defects. None of this is Tegra specific and affects all vendors equally.
Now Hafnium, during GIC init loops over the GICR to find the last frame. But it also reads the MPIDR from the GICR_TYPER register and tries to find the corresponding value in the manifest. If the MPIDR does not exist in the manifest, it asserts.
This behavior should be fixed to avoid asserting for "missing" MPIDR values in the manifest. The patches NVIDIA posted, fix this behavior, and decouple the CPU topology from the GIC initialization. The TF-A code, although similar, does not expect the MPIDR to match with anything and does not assert.
Another way to solve this problem would be to add MPIDR to match the GICR frames to the spmc_manifest. But that would be an incorrect picture of the topology.
-Varun
-----Original Message----- From: Olivier Deprez Olivier.Deprez@arm.com Sent: Monday, 14 February 2022 6:30 PM To: hafnium@lists.trustedfirmware.org; Varun Wadekar vwadekar@nvidia.com Cc: Bo Yan byan@nvidia.com Subject: Re: GICR discovery
External email: Use caution opening links or attachments
Hi Varun,
Out of curiosity, I'm trying to understand how far this change is generic or a specific chipset workaround? In other words, how does this deviate from the Arm architecture?... Is this a common practise to disable PEs but leave their MPIDR identification into the GIC?
The redistributor probing implemented in Hafnium is very much the same pattern as in the TF-A GIC driver. In the Hafnium case, would it be nicer to walk the list of cpus declared in the spmc manifest, and match the corresponding MPIDR from redistributor?
Regards, Olivier.
________________________________________ From: Varun Wadekar via Hafnium hafnium@lists.trustedfirmware.org Sent: 14 February 2022 19:04 To: hafnium@lists.trustedfirmware.org Cc: Bo Yan Subject: [Hafnium] GICR discovery
Hello team,
Currently the amount of memory allocated for GICR frames is determined by the number of supported CPUs. However, the GIC redistributor might have more frames than the number of PEs. In such a case, it is possible that the core index constructed from GICR_TYPER register points to a non-existent PE. For such a case, the GIC discovery and init sequence should move to the next redistributor frame. Today, the code asserts if there are holes in the CPU topology or if GICR_FRAMES > MAX_CPUS.
Downstream Tegra platforms provide more GICR frames than number of CPUs and require the support posted to gerrit [1]. Request the team to review and post feedback.
Thanks.
[1] topic:"gicv3-gicr-frames" (status:open OR status:merged) * Gerrit Code Review (trustedfirmware.org)https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.trustedfirmware.org%2Fq%2Ftopic%3A%2522gicv3-gicr-frames%2522&data=04%7C01%7Cvwadekar%40nvidia.com%7C7392e2d796dc4a75346e08d9efe8088b%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637804602127338547%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Pfxazbt98Nk%2FuW7LwN11o8ik2hi94oCq8lH22oaJ5B4%3D&reserved=0+(status:open%20OR%20status:merged) -- Hafnium mailing list -- hafnium@lists.trustedfirmware.org To unsubscribe send an email to hafnium-leave@lists.trustedfirmware.org
Hi Varun,
Thanks for this clarification, it makes sense.
Another way to solve this problem would be to add MPIDR to match the GICR frames to the spmc_manifest. But that would be an incorrect picture of the topology.
The manifest already provides the MPIDR for each core. Isn't it possible to walk the cpu list from manifest and match corresponding MPIDRs from the redistributor? Or is this what you mean above? Not pushing hard for this alternative though.
Regards, Olivier.
________________________________________ From: Varun Wadekar vwadekar@nvidia.com Sent: 15 February 2022 09:22 To: Olivier Deprez; hafnium@lists.trustedfirmware.org; Varun Wadekar Cc: Bo Yan Subject: RE: GICR discovery
Hi,
If we look at the GIC spec, the last GICR frame is marked with GICR_TYPER bit 4 set to '1'. The spec does not mandate that GICR frames must match the number of CPUs. Plus, there can be holes in the CPU topology due to manufacturing defects. None of this is Tegra specific and affects all vendors equally.
Now Hafnium, during GIC init loops over the GICR to find the last frame. But it also reads the MPIDR from the GICR_TYPER register and tries to find the corresponding value in the manifest. If the MPIDR does not exist in the manifest, it asserts.
This behavior should be fixed to avoid asserting for "missing" MPIDR values in the manifest. The patches NVIDIA posted, fix this behavior, and decouple the CPU topology from the GIC initialization. The TF-A code, although similar, does not expect the MPIDR to match with anything and does not assert.
Another way to solve this problem would be to add MPIDR to match the GICR frames to the spmc_manifest. But that would be an incorrect picture of the topology.
-Varun
-----Original Message----- From: Olivier Deprez Olivier.Deprez@arm.com Sent: Monday, 14 February 2022 6:30 PM To: hafnium@lists.trustedfirmware.org; Varun Wadekar vwadekar@nvidia.com Cc: Bo Yan byan@nvidia.com Subject: Re: GICR discovery
External email: Use caution opening links or attachments
Hi Varun,
Out of curiosity, I'm trying to understand how far this change is generic or a specific chipset workaround? In other words, how does this deviate from the Arm architecture?... Is this a common practise to disable PEs but leave their MPIDR identification into the GIC?
The redistributor probing implemented in Hafnium is very much the same pattern as in the TF-A GIC driver. In the Hafnium case, would it be nicer to walk the list of cpus declared in the spmc manifest, and match the corresponding MPIDR from redistributor?
Regards, Olivier.
________________________________________ From: Varun Wadekar via Hafnium hafnium@lists.trustedfirmware.org Sent: 14 February 2022 19:04 To: hafnium@lists.trustedfirmware.org Cc: Bo Yan Subject: [Hafnium] GICR discovery
Hello team,
Currently the amount of memory allocated for GICR frames is determined by the number of supported CPUs. However, the GIC redistributor might have more frames than the number of PEs. In such a case, it is possible that the core index constructed from GICR_TYPER register points to a non-existent PE. For such a case, the GIC discovery and init sequence should move to the next redistributor frame. Today, the code asserts if there are holes in the CPU topology or if GICR_FRAMES > MAX_CPUS.
Downstream Tegra platforms provide more GICR frames than number of CPUs and require the support posted to gerrit [1]. Request the team to review and post feedback.
Thanks.
[1] topic:"gicv3-gicr-frames" (status:open OR status:merged) * Gerrit Code Review (trustedfirmware.org)https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.trustedfirmware.org%2Fq%2Ftopic%3A%2522gicv3-gicr-frames%2522&data=04%7C01%7Cvwadekar%40nvidia.com%7C7392e2d796dc4a75346e08d9efe8088b%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637804602127338547%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Pfxazbt98Nk%2FuW7LwN11o8ik2hi94oCq8lH22oaJ5B4%3D&reserved=0+(status:open%20OR%20status:merged) -- Hafnium mailing list -- hafnium@lists.trustedfirmware.org To unsubscribe send an email to hafnium-leave@lists.trustedfirmware.org
Hi,
Isn't it possible to walk the cpu list from manifest and match corresponding MPIDRs from the redistributor?
It is the other way round. During GIC init, we check if the MPIDR read from GICR_TYPER is available in the manifest.
The manifest will provide MPIDR for the cores that are present in the system. To get past the GICR assert and init failure, we will have to add MPIDR for CPUs that do not exist. That would be an incorrect picture of the CPU topology. I believe the right approach is to modify the GIC init code to accommodate this scenario.
-Varun
-----Original Message----- From: Olivier Deprez Olivier.Deprez@arm.com Sent: Wednesday, 16 February 2022 8:42 AM To: Varun Wadekar vwadekar@nvidia.com; hafnium@lists.trustedfirmware.org Cc: Bo Yan byan@nvidia.com Subject: Re: GICR discovery
External email: Use caution opening links or attachments
Hi Varun,
Thanks for this clarification, it makes sense.
Another way to solve this problem would be to add MPIDR to match the GICR frames to the spmc_manifest. But that would be an incorrect picture of the topology.
The manifest already provides the MPIDR for each core. Isn't it possible to walk the cpu list from manifest and match corresponding MPIDRs from the redistributor? Or is this what you mean above? Not pushing hard for this alternative though.
Regards, Olivier.
________________________________________ From: Varun Wadekar vwadekar@nvidia.com Sent: 15 February 2022 09:22 To: Olivier Deprez; hafnium@lists.trustedfirmware.org; Varun Wadekar Cc: Bo Yan Subject: RE: GICR discovery
Hi,
If we look at the GIC spec, the last GICR frame is marked with GICR_TYPER bit 4 set to '1'. The spec does not mandate that GICR frames must match the number of CPUs. Plus, there can be holes in the CPU topology due to manufacturing defects. None of this is Tegra specific and affects all vendors equally.
Now Hafnium, during GIC init loops over the GICR to find the last frame. But it also reads the MPIDR from the GICR_TYPER register and tries to find the corresponding value in the manifest. If the MPIDR does not exist in the manifest, it asserts.
This behavior should be fixed to avoid asserting for "missing" MPIDR values in the manifest. The patches NVIDIA posted, fix this behavior, and decouple the CPU topology from the GIC initialization. The TF-A code, although similar, does not expect the MPIDR to match with anything and does not assert.
Another way to solve this problem would be to add MPIDR to match the GICR frames to the spmc_manifest. But that would be an incorrect picture of the topology.
-Varun
-----Original Message----- From: Olivier Deprez Olivier.Deprez@arm.com Sent: Monday, 14 February 2022 6:30 PM To: hafnium@lists.trustedfirmware.org; Varun Wadekar vwadekar@nvidia.com Cc: Bo Yan byan@nvidia.com Subject: Re: GICR discovery
External email: Use caution opening links or attachments
Hi Varun,
Out of curiosity, I'm trying to understand how far this change is generic or a specific chipset workaround? In other words, how does this deviate from the Arm architecture?... Is this a common practise to disable PEs but leave their MPIDR identification into the GIC?
The redistributor probing implemented in Hafnium is very much the same pattern as in the TF-A GIC driver. In the Hafnium case, would it be nicer to walk the list of cpus declared in the spmc manifest, and match the corresponding MPIDR from redistributor?
Regards, Olivier.
________________________________________ From: Varun Wadekar via Hafnium hafnium@lists.trustedfirmware.org Sent: 14 February 2022 19:04 To: hafnium@lists.trustedfirmware.org Cc: Bo Yan Subject: [Hafnium] GICR discovery
Hello team,
Currently the amount of memory allocated for GICR frames is determined by the number of supported CPUs. However, the GIC redistributor might have more frames than the number of PEs. In such a case, it is possible that the core index constructed from GICR_TYPER register points to a non-existent PE. For such a case, the GIC discovery and init sequence should move to the next redistributor frame. Today, the code asserts if there are holes in the CPU topology or if GICR_FRAMES > MAX_CPUS.
Downstream Tegra platforms provide more GICR frames than number of CPUs and require the support posted to gerrit [1]. Request the team to review and post feedback.
Thanks.
[1] topic:"gicv3-gicr-frames" (status:open OR status:merged) * Gerrit Code Review (trustedfirmware.org)https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.trustedfirmware.org%2Fq%2Ftopic%3A%2522gicv3-gicr-frames%2522&data=04%7C01%7Cvwadekar%40nvidia.com%7Cd0289b8b992743347a2d08d9f1283a73%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637805977350020595%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2FeE8ebr6gG5grcTgvMT99JfoC8LHgLFFj1uczhlQ8a8%3D&reserved=0+(status:open%20OR%20status:merged) -- Hafnium mailing list -- hafnium@lists.trustedfirmware.org To unsubscribe send an email to hafnium-leave@lists.trustedfirmware.org
hafnium@lists.trustedfirmware.org
-
Olivier Deprez -
Varun Wadekar