[edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias

Marcin Wojtas posted 13 patches 7 years, 2 months ago
There is a newer version of this series
[edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias
Posted by Marcin Wojtas 7 years, 2 months ago
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The GIC architecture mandates that the CPU interface, which consists
of 2 consecutive 4 KB frames, can be mapped using separate mappings.
Since this is problematic on 64 KB pages, the MMU-400 aliases each
frame 16 times, and the two consecutive frames can be found at offset
0xf000. This patch is intended to expose correct GICC alias via
MADT, once ACPI support is added.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada/Armada.dsc.inc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
index 5071bd5..bd2336f 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -263,7 +263,14 @@
 
   # ARM Generic Interrupt Controller
   gArmTokenSpaceGuid.PcdGicDistributorBase|0xF0210000
-  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF0220000
+
+  #
+  # NOTE: the GIC architecture mandates that the CPU interface, which consists
+  # of 2 consecutive 4 KB frames, can be mapped using separate mappings.
+  # Since this is problematic on 64 KB pages, the MMU-400 aliases each frame
+  # 16 times, and the two consecutive frames can be found at offset 0xf000
+  #
+  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF022F000
 
   # ARM Architectural Timer Support
   gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|25000000
-- 
1.8.3.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias
Posted by Leif Lindholm 7 years, 2 months ago
On Mon, Oct 09, 2017 at 07:00:57PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The GIC architecture mandates that the CPU interface, which consists
> of 2 consecutive 4 KB frames, can be mapped using separate mappings.
> Since this is problematic on 64 KB pages, the MMU-400 aliases each
> frame 16 times, and the two consecutive frames can be found at offset
> 0xf000. This patch is intended to expose correct GICC alias via
> MADT, once ACPI support is added.

I'm afraid I don't quite understand this message.

The change seems to be that the InterfaceBase moves from the first 4KB
alias inside a 64KB page to the last alias within the same page.
That seems valid, but I don't see how it resolves anything described
in this message?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Armada/Armada.dsc.inc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
> index 5071bd5..bd2336f 100644
> --- a/Platform/Marvell/Armada/Armada.dsc.inc
> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
> @@ -263,7 +263,14 @@
>  
>    # ARM Generic Interrupt Controller
>    gArmTokenSpaceGuid.PcdGicDistributorBase|0xF0210000
> -  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF0220000
> +
> +  #
> +  # NOTE: the GIC architecture mandates that the CPU interface, which consists
> +  # of 2 consecutive 4 KB frames, can be mapped using separate mappings.
> +  # Since this is problematic on 64 KB pages, the MMU-400 aliases each frame
> +  # 16 times, and the two consecutive frames can be found at offset 0xf000
> +  #
> +  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xF022F000
>  
>    # ARM Architectural Timer Support
>    gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|25000000
> -- 
> 1.8.3.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias
Posted by Marcin Wojtas 7 years, 2 months ago
Hi Ard,

2017-10-10 16:53 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Mon, Oct 09, 2017 at 07:00:57PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> The GIC architecture mandates that the CPU interface, which consists
>> of 2 consecutive 4 KB frames, can be mapped using separate mappings.
>> Since this is problematic on 64 KB pages, the MMU-400 aliases each
>> frame 16 times, and the two consecutive frames can be found at offset
>> 0xf000. This patch is intended to expose correct GICC alias via
>> MADT, once ACPI support is added.
>
> I'm afraid I don't quite understand this message.
>
> The change seems to be that the InterfaceBase moves from the first 4KB
> alias inside a 64KB page to the last alias within the same page.
> That seems valid, but I don't see how it resolves anything described
> in this message?
>

Can you recall what issue was fixed thanks to this patch?

Best regards,
Marcin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias
Posted by Ard Biesheuvel 7 years, 2 months ago
On 10 October 2017 at 15:56, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi Ard,
>
> 2017-10-10 16:53 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
>> On Mon, Oct 09, 2017 at 07:00:57PM +0200, Marcin Wojtas wrote:
>>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>>> The GIC architecture mandates that the CPU interface, which consists
>>> of 2 consecutive 4 KB frames, can be mapped using separate mappings.
>>> Since this is problematic on 64 KB pages, the MMU-400 aliases each
>>> frame 16 times, and the two consecutive frames can be found at offset
>>> 0xf000. This patch is intended to expose correct GICC alias via
>>> MADT, once ACPI support is added.
>>
>> I'm afraid I don't quite understand this message.
>>
>> The change seems to be that the InterfaceBase moves from the first 4KB
>> alias inside a 64KB page to the last alias within the same page.
>> That seems valid, but I don't see how it resolves anything described
>> in this message?
>>

Because now, GICC + 4 KB will point at the second frame, and so the
two frames appear adjacently, and precisely 4 KB apart. And at the
same time, they are still covered by distinct 64 KB pages so it even
works when running the OS with 64k pages.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 08/13] Marvell/Armada: Modify GICC alias
Posted by Leif Lindholm 7 years, 2 months ago
On Tue, Oct 10, 2017 at 09:45:29PM +0100, Ard Biesheuvel wrote:
> On 10 October 2017 at 15:56, Marcin Wojtas <mw@semihalf.com> wrote:
> > Hi Ard,
> >
> > 2017-10-10 16:53 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> >> On Mon, Oct 09, 2017 at 07:00:57PM +0200, Marcin Wojtas wrote:
> >>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>>
> >>> The GIC architecture mandates that the CPU interface, which consists
> >>> of 2 consecutive 4 KB frames, can be mapped using separate mappings.
> >>> Since this is problematic on 64 KB pages, the MMU-400 aliases each
> >>> frame 16 times, and the two consecutive frames can be found at offset
> >>> 0xf000. This patch is intended to expose correct GICC alias via
> >>> MADT, once ACPI support is added.
> >>
> >> I'm afraid I don't quite understand this message.
> >>
> >> The change seems to be that the InterfaceBase moves from the first 4KB
> >> alias inside a 64KB page to the last alias within the same page.
> >> That seems valid, but I don't see how it resolves anything described
> >> in this message?
> >>
> 
> Because now, GICC + 4 KB will point at the second frame, and so the
> two frames appear adjacently, and precisely 4 KB apart. And at the
> same time, they are still covered by distinct 64 KB pages so it even
> works when running the OS with 64k pages.

Right, I was thinking it might be something like that, but I didn't
get that from the patch - commit message _or_ comment.

Maybe add something like "Use the last alias from the first series of
aliases as the base address, so that the first frame from the second
series becomes directly adjacent, whilst remaining covered by a
separate 64kB page"?

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel