Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-29 Thread Laszlo Ersek 
On 01/29/19 12:19, xinliang wrote:
> 
> 
> On 2019/1/29 14:47, Tan Xiaojun wrote:
>> +cc Xinliang
>>
>> Sorry, I didn't react to what you discussed at the beginning, because
>> this
>> problem is mainly handled by Xinliang.
>>
>> The host we used for testing is Huawei D06 (AArch64), and its CPU chip is
>> 1620 (self-developed chip that follows the arm v8.2 specification).
>> Its vmid
>> is 16 bit.
>>
>> Thanks.
>> Xiaojun.
>>
>> On 2019/1/28 21:46, Mark Rutland wrote:
>>> On Mon, Jan 28, 2019 at 09:09:26PM +0800, Tan Xiaojun wrote:
 On 2019/1/28 19:54, Laszlo Ersek wrote:
> On 01/28/19 11:46, Mark Rutland wrote:
>> On Wed, Jan 23, 2019 at 10:54:56AM +0100, Laszlo Ersek wrote:
>>> And even on the original (unspecified) hardware, the same binary
>>> works
>>> frequently. My understanding is that there are five VMs executing
>>> reboot
>>> loops in parallel, on the same host, and 4 out of 5 may hit the
>>> issue in
>>> a reasonable time period (300 reboots or so).
>> Interesting.
>>
>> Do you happen to know how many VMID bits the host has? If it has
>> an 8-bit VMID,
>> this could be indicative of some problem upon overflow.
> I'll let Tan Xiaojun (CC'd) answer this questions.
>
>> Can you point us at the host kernel?
> In the report, Tan Xiaojun wrote "4.18.0-48.el8.aarch64"; I guess that
> information is mostly useless in an upstream discussion.
> Unfortunately,
> I couldn't reproduce the symptom at all (I know nothing about the
> hardware in question), so I can't myself retest with an upstream host
> kernel.
 I don't understand, what do you want me to do? What is the specific
 problem?
>>> Could you let us know which CPU/system you've seen this issue with?
>>>
>>> ... and what the value of ID_AA64MMFR1_EL1.VMIDBits is?
> 
> Hi, our hardware is D06 board which compatible with armv8.1. So it
> should be 16-bit VMID.
> 
> BTW, we now only reproduce this issue on guest without shim, such as
> suse SLE15-SP1 guest which is easy to reproduce. This founding not sure
> could help for this issue.

Unfortunately, it doesn't help without access to the host -- I used the
exact same guest installer ISO from the original report, and failed to
reproduce the symptom.

... Since we've been discussing this for a while in public too, can
someone from Huawei please authorize Red Hat to open up
 to the public? If
you agree, please comment so on the BZ itself.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Mark Rutland 
On Mon, Jan 28, 2019 at 09:09:26PM +0800, Tan Xiaojun wrote:
> On 2019/1/28 19:54, Laszlo Ersek wrote:
> > On 01/28/19 11:46, Mark Rutland wrote:
> >> On Wed, Jan 23, 2019 at 10:54:56AM +0100, Laszlo Ersek wrote:
> >>> And even on the original (unspecified) hardware, the same binary works
> >>> frequently. My understanding is that there are five VMs executing reboot
> >>> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> >>> a reasonable time period (300 reboots or so).
> >>
> >> Interesting.
> >>
> >> Do you happen to know how many VMID bits the host has? If it has an 8-bit 
> >> VMID,
> >> this could be indicative of some problem upon overflow.
> > 
> > I'll let Tan Xiaojun (CC'd) answer this questions.
> > 
> >> Can you point us at the host kernel?
> > 
> > In the report, Tan Xiaojun wrote "4.18.0-48.el8.aarch64"; I guess that
> > information is mostly useless in an upstream discussion. Unfortunately,
> > I couldn't reproduce the symptom at all (I know nothing about the
> > hardware in question), so I can't myself retest with an upstream host
> > kernel.
> 
> I don't understand, what do you want me to do? What is the specific problem?

Could you let us know which CPU/system you've seen this issue with?

... and what the value of ID_AA64MMFR1_EL1.VMIDBits is?

Thanks,
Mark.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Laszlo Ersek 
On 01/28/19 14:09, Tan Xiaojun wrote:
> On 2019/1/28 19:54, Laszlo Ersek wrote:
>> On 01/28/19 11:46, Mark Rutland wrote:
>>> On Wed, Jan 23, 2019 at 10:54:56AM +0100, Laszlo Ersek wrote:
 On 01/23/19 10:26, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
>> On 01/22/19 16:37, Ard Biesheuvel wrote:

>>> Is SetUefiImageMemoryAttributes() being
>>> called to remap the memory R-X ?
>>
>> No, it is not; the grub binary in question doesn't have the required
>> section alignment (... I hope at least that that's what your question
>> refers to):
>>
>>> ProtectUefiImageCommon - 0x3E6C54C0
>>>   - 0x00013BEEF000 - 0x00030600
>>>   ProtectUefiImageCommon - Section Alignment(0x200) is
>> incorrect  
>
> This is puzzling, given that the exact same binary works on Mustang.

 And even on the original (unspecified) hardware, the same binary works
 frequently. My understanding is that there are five VMs executing reboot
 loops in parallel, on the same host, and 4 out of 5 may hit the issue in
 a reasonable time period (300 reboots or so).
>>>
>>> Interesting.
>>>
>>> Do you happen to know how many VMID bits the host has? If it has an 8-bit 
>>> VMID,
>>> this could be indicative of some problem upon overflow.
>>
>> I'll let Tan Xiaojun (CC'd) answer this questions.
>>
>>> Can you point us at the host kernel?
>>
>> In the report, Tan Xiaojun wrote "4.18.0-48.el8.aarch64"; I guess that
>> information is mostly useless in an upstream discussion. Unfortunately,
>> I couldn't reproduce the symptom at all (I know nothing about the
>> hardware in question), so I can't myself retest with an upstream host
>> kernel.
>>
> 
> I don't understand, what do you want me to do? What is the specific problem?

Sorry, I was unclear. Primarily, please see Mark's explanation.

Secondarily, my point was that the upstream community could help more if
the symptom reproduced on a pristine upstream host kernel. Given that I
don't have access to your hardware that presents the symptom, plus that
the symptom doesn't reproduce on hardware that I do have access to,
using the downstream kernel that you reported, only you can attempt to
repro the issue with an upstream kernel (and then please report the
findings here).

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Laszlo Ersek 
On 01/28/19 11:46, Mark Rutland wrote:
> On Wed, Jan 23, 2019 at 10:54:56AM +0100, Laszlo Ersek wrote:
>> On 01/23/19 10:26, Ard Biesheuvel wrote:
>>> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
 On 01/22/19 16:37, Ard Biesheuvel wrote:
>>
> Is SetUefiImageMemoryAttributes() being
> called to remap the memory R-X ?

 No, it is not; the grub binary in question doesn't have the required
 section alignment (... I hope at least that that's what your question
 refers to):

> ProtectUefiImageCommon - 0x3E6C54C0
>   - 0x00013BEEF000 - 0x00030600
>   ProtectUefiImageCommon - Section Alignment(0x200) is
 incorrect  
>>>
>>> This is puzzling, given that the exact same binary works on Mustang.
>>
>> And even on the original (unspecified) hardware, the same binary works
>> frequently. My understanding is that there are five VMs executing reboot
>> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
>> a reasonable time period (300 reboots or so).
> 
> Interesting.
> 
> Do you happen to know how many VMID bits the host has? If it has an 8-bit 
> VMID,
> this could be indicative of some problem upon overflow.

I'll let Tan Xiaojun (CC'd) answer this questions.

> Can you point us at the host kernel?

In the report, Tan Xiaojun wrote "4.18.0-48.el8.aarch64"; I guess that
information is mostly useless in an upstream discussion. Unfortunately,
I couldn't reproduce the symptom at all (I know nothing about the
hardware in question), so I can't myself retest with an upstream host
kernel.

Thank you!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Mark Rutland 
On Wed, Jan 23, 2019 at 10:54:56AM +0100, Laszlo Ersek wrote:
> On 01/23/19 10:26, Ard Biesheuvel wrote:
> > On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
> >> On 01/22/19 16:37, Ard Biesheuvel wrote:
> 
> >>> Is SetUefiImageMemoryAttributes() being
> >>> called to remap the memory R-X ?
> >>
> >> No, it is not; the grub binary in question doesn't have the required
> >> section alignment (... I hope at least that that's what your question
> >> refers to):
> >>
> >>> ProtectUefiImageCommon - 0x3E6C54C0
> >>>   - 0x00013BEEF000 - 0x00030600
> >>>   ProtectUefiImageCommon - Section Alignment(0x200) is
> >> incorrect  
> > 
> > This is puzzling, given that the exact same binary works on Mustang.
> 
> And even on the original (unspecified) hardware, the same binary works
> frequently. My understanding is that there are five VMs executing reboot
> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> a reasonable time period (300 reboots or so).

Interesting.

Do you happen to know how many VMID bits the host has? If it has an 8-bit VMID,
this could be indicative of some problem upon overflow.

Can you point us at the host kernel?

Thanks,
Mark.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Ard Biesheuvel 
On Mon, 28 Jan 2019 at 11:23, Mark Rutland  wrote:
>
> On Wed, Jan 23, 2019 at 03:02:14PM +0100, Ard Biesheuvel wrote:
> > On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek  wrote:
> > >
> > > On 01/23/19 10:26, Ard Biesheuvel wrote:
> > > > On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
> > > >> On 01/22/19 16:37, Ard Biesheuvel wrote:
> > >
> > > >>> Is SetUefiImageMemoryAttributes() being
> > > >>> called to remap the memory R-X ?
> > > >>
> > > >> No, it is not; the grub binary in question doesn't have the required
> > > >> section alignment (... I hope at least that that's what your question
> > > >> refers to):
> > > >>
> > > >>> ProtectUefiImageCommon - 0x3E6C54C0
> > > >>>   - 0x00013BEEF000 - 0x00030600
> > > >>>   ProtectUefiImageCommon - Section Alignment(0x200) is
> > > >> incorrect  
> > > >>
> > > >
> > > > This is puzzling, given that the exact same binary works on Mustang.
> > >
> > > And even on the original (unspecified) hardware, the same binary works
> > > frequently. My understanding is that there are five VMs executing reboot
> > > loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> > > a reasonable time period (300 reboots or so).
> > >
> > > > So when loaded, GRUB should cover the following regions:
> > > >
> > > > 0x13beef - 0x13bf00 (0x11000)
> > > > 0x13bf00 - 0x13bf01f600 (0x1f600)
> > > >
> > > > where neither covers a 2 MB block fully, which means that the TLB
> > > > entry that we are hitting is stale.
> > > >
> > > > Since ProtectUefiImageCommon() does not do anything in this case, the
> > > > stale translation must be the result of
> > > > PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> > > > permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> > > > we don't flush the TLBs correctly after updating the permissions when
> > > > converting the memory from EfiConventionalMemory to EfiLoaderCode
> > > >
> > > > Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
> > >
> > > Yes, we have
> > >
> > > ArmVirtPkg/ArmVirt.dsc.inc:
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
> > >
> > > from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
> > > protection for all platforms", 2017-03-01).
> > >
> > > The binary is from the RPM
> > > "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
> > > is basically upstream ee3198e672e2 plus a small number of backports and
> > > downstream customizations.
> > >
> >
> > This might help:
> >
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > index b7173e00b039..4c0b4b4efbd5 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> > @@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
> >
> >  ASM_FUNC(ArmInvalidateTlb)
> > EL1_OR_EL2_OR_EL3(x0)
> > -1: tlbi  vmalle1
> > +1: tlbi  vmalle1is
> > b 4f
> >  2: tlbi  alle2
> > b 4f
> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> > index 90192df24f55..d54b1c19accf 100644
> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> > @@ -34,7 +34,7 @@
> >
> >// flush the TLBs
> >.if   \el == 1
> > -  tlbi  vmalle1
> > +  tlbi  vmalle1is
> >.else
> >tlbi  alle\el
> >.endif
>
> Assuming that hardware is working correctly, this change shouldn't be
> necessary.
>
> KVM sets HCR_EL2.FB, so all TLBI ops will behave as their *IS variant.
> Likewise it sets HCR_EL2.BSU, so barriers apply to the inner shareable domain 
> too.
>
> On bare-metal, NSH should be sufficient.
>

Ah wonderful, thanks for clarifying.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-28 Thread Mark Rutland 
On Wed, Jan 23, 2019 at 03:02:14PM +0100, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek  wrote:
> >
> > On 01/23/19 10:26, Ard Biesheuvel wrote:
> > > On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
> > >> On 01/22/19 16:37, Ard Biesheuvel wrote:
> >
> > >>> Is SetUefiImageMemoryAttributes() being
> > >>> called to remap the memory R-X ?
> > >>
> > >> No, it is not; the grub binary in question doesn't have the required
> > >> section alignment (... I hope at least that that's what your question
> > >> refers to):
> > >>
> > >>> ProtectUefiImageCommon - 0x3E6C54C0
> > >>>   - 0x00013BEEF000 - 0x00030600
> > >>>   ProtectUefiImageCommon - Section Alignment(0x200) is
> > >> incorrect  
> > >>
> > >
> > > This is puzzling, given that the exact same binary works on Mustang.
> >
> > And even on the original (unspecified) hardware, the same binary works
> > frequently. My understanding is that there are five VMs executing reboot
> > loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> > a reasonable time period (300 reboots or so).
> >
> > > So when loaded, GRUB should cover the following regions:
> > >
> > > 0x13beef - 0x13bf00 (0x11000)
> > > 0x13bf00 - 0x13bf01f600 (0x1f600)
> > >
> > > where neither covers a 2 MB block fully, which means that the TLB
> > > entry that we are hitting is stale.
> > >
> > > Since ProtectUefiImageCommon() does not do anything in this case, the
> > > stale translation must be the result of
> > > PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> > > permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> > > we don't flush the TLBs correctly after updating the permissions when
> > > converting the memory from EfiConventionalMemory to EfiLoaderCode
> > >
> > > Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
> >
> > Yes, we have
> >
> > ArmVirtPkg/ArmVirt.dsc.inc:
> > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
> >
> > from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
> > protection for all platforms", 2017-03-01).
> >
> > The binary is from the RPM
> > "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
> > is basically upstream ee3198e672e2 plus a small number of backports and
> > downstream customizations.
> >
> 
> This might help:
> 
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index b7173e00b039..4c0b4b4efbd5 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
> 
>  ASM_FUNC(ArmInvalidateTlb)
> EL1_OR_EL2_OR_EL3(x0)
> -1: tlbi  vmalle1
> +1: tlbi  vmalle1is
> b 4f
>  2: tlbi  alle2
> b 4f
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index 90192df24f55..d54b1c19accf 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -34,7 +34,7 @@
> 
>// flush the TLBs
>.if   \el == 1
> -  tlbi  vmalle1
> +  tlbi  vmalle1is
>.else
>tlbi  alle\el
>.endif

Assuming that hardware is working correctly, this change shouldn't be
necessary.

KVM sets HCR_EL2.FB, so all TLBI ops will behave as their *IS variant.
Likewise it sets HCR_EL2.BSU, so barriers apply to the inner shareable domain 
too.

On bare-metal, NSH should be sufficient.

Thanks,
Mark.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-23 Thread Laszlo Ersek 
On 01/23/19 15:02, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek  wrote:
>>
>> On 01/23/19 10:26, Ard Biesheuvel wrote:
>>> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
 On 01/22/19 16:37, Ard Biesheuvel wrote:
>>
> Is SetUefiImageMemoryAttributes() being
> called to remap the memory R-X ?

 No, it is not; the grub binary in question doesn't have the required
 section alignment (... I hope at least that that's what your question
 refers to):

> ProtectUefiImageCommon - 0x3E6C54C0
>   - 0x00013BEEF000 - 0x00030600
>   ProtectUefiImageCommon - Section Alignment(0x200) is
 incorrect  

>>>
>>> This is puzzling, given that the exact same binary works on Mustang.
>>
>> And even on the original (unspecified) hardware, the same binary works
>> frequently. My understanding is that there are five VMs executing reboot
>> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
>> a reasonable time period (300 reboots or so).
>>
>>> So when loaded, GRUB should cover the following regions:
>>>
>>> 0x13beef - 0x13bf00 (0x11000)
>>> 0x13bf00 - 0x13bf01f600 (0x1f600)
>>>
>>> where neither covers a 2 MB block fully, which means that the TLB
>>> entry that we are hitting is stale.
>>>
>>> Since ProtectUefiImageCommon() does not do anything in this case, the
>>> stale translation must be the result of
>>> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
>>> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
>>> we don't flush the TLBs correctly after updating the permissions when
>>> converting the memory from EfiConventionalMemory to EfiLoaderCode
>>>
>>> Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
>>
>> Yes, we have
>>
>> ArmVirtPkg/ArmVirt.dsc.inc:
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
>>
>> from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
>> protection for all platforms", 2017-03-01).
>>
>> The binary is from the RPM
>> "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
>> is basically upstream ee3198e672e2 plus a small number of backports and
>> downstream customizations.
>>
> 
> This might help:
> 
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index b7173e00b039..4c0b4b4efbd5 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)
> 
>  ASM_FUNC(ArmInvalidateTlb)
> EL1_OR_EL2_OR_EL3(x0)
> -1: tlbi  vmalle1
> +1: tlbi  vmalle1is
> b 4f
>  2: tlbi  alle2
> b 4f
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> index 90192df24f55..d54b1c19accf 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
> @@ -34,7 +34,7 @@
> 
>// flush the TLBs
>.if   \el == 1
> -  tlbi  vmalle1
> +  tlbi  vmalle1is
>.else
>tlbi  alle\el
>.endif
> 

"Invalidate all EL1&0 regime stage 1 TLB entries for the current VMID
>>>on all PEs in the same Inner Shareable domain<<<".

I'll soon try to build a new pkg with this applied, for testing by the
reporter. Many thanks!

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-23 Thread Ard Biesheuvel 
On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek  wrote:
>
> On 01/23/19 10:26, Ard Biesheuvel wrote:
> > On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
> >> On 01/22/19 16:37, Ard Biesheuvel wrote:
>
> >>> Is SetUefiImageMemoryAttributes() being
> >>> called to remap the memory R-X ?
> >>
> >> No, it is not; the grub binary in question doesn't have the required
> >> section alignment (... I hope at least that that's what your question
> >> refers to):
> >>
> >>> ProtectUefiImageCommon - 0x3E6C54C0
> >>>   - 0x00013BEEF000 - 0x00030600
> >>>   ProtectUefiImageCommon - Section Alignment(0x200) is
> >> incorrect  
> >>
> >
> > This is puzzling, given that the exact same binary works on Mustang.
>
> And even on the original (unspecified) hardware, the same binary works
> frequently. My understanding is that there are five VMs executing reboot
> loops in parallel, on the same host, and 4 out of 5 may hit the issue in
> a reasonable time period (300 reboots or so).
>
> > So when loaded, GRUB should cover the following regions:
> >
> > 0x13beef - 0x13bf00 (0x11000)
> > 0x13bf00 - 0x13bf01f600 (0x1f600)
> >
> > where neither covers a 2 MB block fully, which means that the TLB
> > entry that we are hitting is stale.
> >
> > Since ProtectUefiImageCommon() does not do anything in this case, the
> > stale translation must be the result of
> > PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> > permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> > we don't flush the TLBs correctly after updating the permissions when
> > converting the memory from EfiConventionalMemory to EfiLoaderCode
> >
> > Are you using the default value for PcdDxeNxMemoryProtectionPolicy?
>
> Yes, we have
>
> ArmVirtPkg/ArmVirt.dsc.inc:
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1
>
> from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
> protection for all platforms", 2017-03-01).
>
> The binary is from the RPM
> "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
> is basically upstream ee3198e672e2 plus a small number of backports and
> downstream customizations.
>

This might help:

diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index b7173e00b039..4c0b4b4efbd5 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry)

 ASM_FUNC(ArmInvalidateTlb)
EL1_OR_EL2_OR_EL3(x0)
-1: tlbi  vmalle1
+1: tlbi  vmalle1is
b 4f
 2: tlbi  alle2
b 4f
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
index 90192df24f55..d54b1c19accf 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
@@ -34,7 +34,7 @@

   // flush the TLBs
   .if   \el == 1
-  tlbi  vmalle1
+  tlbi  vmalle1is
   .else
   tlbi  alle\el
   .endif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-23 Thread Laszlo Ersek 
On 01/23/19 10:26, Ard Biesheuvel wrote:
> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
>> On 01/22/19 16:37, Ard Biesheuvel wrote:

>>> Is SetUefiImageMemoryAttributes() being
>>> called to remap the memory R-X ?
>>
>> No, it is not; the grub binary in question doesn't have the required
>> section alignment (... I hope at least that that's what your question
>> refers to):
>>
>>> ProtectUefiImageCommon - 0x3E6C54C0
>>>   - 0x00013BEEF000 - 0x00030600
>>>   ProtectUefiImageCommon - Section Alignment(0x200) is
>> incorrect  
>>
> 
> This is puzzling, given that the exact same binary works on Mustang.

And even on the original (unspecified) hardware, the same binary works
frequently. My understanding is that there are five VMs executing reboot
loops in parallel, on the same host, and 4 out of 5 may hit the issue in
a reasonable time period (300 reboots or so).

> So when loaded, GRUB should cover the following regions:
> 
> 0x13beef - 0x13bf00 (0x11000)
> 0x13bf00 - 0x13bf01f600 (0x1f600)
> 
> where neither covers a 2 MB block fully, which means that the TLB
> entry that we are hitting is stale.
> 
> Since ProtectUefiImageCommon() does not do anything in this case, the
> stale translation must be the result of
> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong
> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or
> we don't flush the TLBs correctly after updating the permissions when
> converting the memory from EfiConventionalMemory to EfiLoaderCode
> 
> Are you using the default value for PcdDxeNxMemoryProtectionPolicy?

Yes, we have

ArmVirtPkg/ArmVirt.dsc.inc:
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1

from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory
protection for all platforms", 2017-03-01).

The binary is from the RPM
"edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which
is basically upstream ee3198e672e2 plus a small number of backports and
downstream customizations.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-23 Thread Ard Biesheuvel 
On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek  wrote:
>
> On 01/22/19 16:37, Ard Biesheuvel wrote:
> > On Tue, 22 Jan 2019 at 16:09, Laszlo Ersek  wrote:
> >>
> >> Performing thread-necromancy on a patch from 2015, which is today known
> >> as commit b7de7e3cab3f. Also CC'ing Christoffer and Marc.
> >>
> >> Question at the bottom.
> >>
> >> On 12/07/15 08:06, Ard Biesheuvel wrote:
> >>> From: "Cohen, Eugene" 
> >>>
> >>> This patch updates the ArmPkg variant of InvalidateInstructionCacheRange 
> >>> to
> >>> flush the data cache only to the point of unification (PoU). This improves
> >>> performance and also allows invalidation in scenarios where it would be
> >>> inappropriate to flush to the point of coherency (like when executing code
> >>> from L2 configured as cache-as-ram).
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Eugene Cohen 
> >>>
> >>> Added AARCH64 and ARM/GCC implementations of the above.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Ard Biesheuvel 
> >>> ---
> >>>  ArmPkg/Include/Library/ArmLib.h| 8 
> >>> +++-
> >>>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 2 +-
> >>>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6 ++
> >>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6 ++
> >>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5 +
> >>>  5 files changed, 25 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/ArmPkg/Include/Library/ArmLib.h 
> >>> b/ArmPkg/Include/Library/ArmLib.h
> >>> index 9622444ec63f..85fa1f600ba9 100644
> >>> --- a/ArmPkg/Include/Library/ArmLib.h
> >>> +++ b/ArmPkg/Include/Library/ArmLib.h
> >>> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
> >>>
> >>>  VOID
> >>>  EFIAPI
> >>> -ArmCleanDataCacheEntryByMVA (
> >>> +ArmCleanDataCacheEntryToPoUByMVA(
> >>>IN  UINTN   Address
> >>>);
> >>>
> >>>  VOID
> >>>  EFIAPI
> >>> +ArmCleanDataCacheEntryByMVA(
> >>> +IN  UINTN   Address
> >>> +);
> >>> +
> >>> +VOID
> >>> +EFIAPI
> >>>  ArmCleanInvalidateDataCacheEntryByMVA (
> >>>IN  UINTN   Address
> >>>);
> >>> diff --git 
> >>> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> >>> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> index feab4497ac1b..1045f9068f4d 100644
> >>> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> >>> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
> >>>IN  UINTN Length
> >>>)
> >>>  {
> >>> -  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryByMVA);
> >>> +  CacheRangeOperation (Address, Length, 
> >>> ArmCleanDataCacheEntryToPoUByMVA);
> >>>ArmInvalidateInstructionCache ();
> >>>return Address;
> >>>  }
> >>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> >>> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> index c530d19e897e..db21f73f0ed7 100644
> >>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >>> @@ -22,6 +22,7 @@
> >>>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> >>> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >>>ret
> >>>
> >>>
> >>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> >>> +  dc  cvau, x0// Clean single data cache line to PoU
> >>> +  ret
> >>> +
> >>> +
> >>>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
> >>>dc  civac, x0   // Clean and invalidate single data cache line
> >>>ret
> >>> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S 
> >>> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> index 5f030d92de31..7de1b11ef818 100644
> >>> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >>> @@ -19,6 +19,7 @@
> >>>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> >>> @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >>>bx  lr
> >>>
> >>>
> >>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> >>> +  mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
> >>> +  bx  lr
> >>> +
> >>> +
> >>>

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-22 Thread Ard Biesheuvel 
On Tue, 22 Jan 2019 at 16:09, Laszlo Ersek  wrote:
>
> Performing thread-necromancy on a patch from 2015, which is today known
> as commit b7de7e3cab3f. Also CC'ing Christoffer and Marc.
>
> Question at the bottom.
>
> On 12/07/15 08:06, Ard Biesheuvel wrote:
> > From: "Cohen, Eugene" 
> >
> > This patch updates the ArmPkg variant of InvalidateInstructionCacheRange to
> > flush the data cache only to the point of unification (PoU). This improves
> > performance and also allows invalidation in scenarios where it would be
> > inappropriate to flush to the point of coherency (like when executing code
> > from L2 configured as cache-as-ram).
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Eugene Cohen 
> >
> > Added AARCH64 and ARM/GCC implementations of the above.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/Include/Library/ArmLib.h| 8 +++-
> >  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 2 +-
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6 ++
> >  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6 ++
> >  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5 +
> >  5 files changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Include/Library/ArmLib.h 
> > b/ArmPkg/Include/Library/ArmLib.h
> > index 9622444ec63f..85fa1f600ba9 100644
> > --- a/ArmPkg/Include/Library/ArmLib.h
> > +++ b/ArmPkg/Include/Library/ArmLib.h
> > @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
> >
> >  VOID
> >  EFIAPI
> > -ArmCleanDataCacheEntryByMVA (
> > +ArmCleanDataCacheEntryToPoUByMVA(
> >IN  UINTN   Address
> >);
> >
> >  VOID
> >  EFIAPI
> > +ArmCleanDataCacheEntryByMVA(
> > +IN  UINTN   Address
> > +);
> > +
> > +VOID
> > +EFIAPI
> >  ArmCleanInvalidateDataCacheEntryByMVA (
> >IN  UINTN   Address
> >);
> > diff --git a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> > b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> > index feab4497ac1b..1045f9068f4d 100644
> > --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> > +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> > @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
> >IN  UINTN Length
> >)
> >  {
> > -  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryByMVA);
> > +  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
> >ArmInvalidateInstructionCache ();
> >return Address;
> >  }
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> > b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > index c530d19e897e..db21f73f0ed7 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > @@ -22,6 +22,7 @@
> >  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> > +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> > @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >ret
> >
> >
> > +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> > +  dc  cvau, x0// Clean single data cache line to PoU
> > +  ret
> > +
> > +
> >  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
> >dc  civac, x0   // Clean and invalidate single data cache line
> >ret
> > diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S 
> > b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> > index 5f030d92de31..7de1b11ef818 100644
> > --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> > +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> > @@ -19,6 +19,7 @@
> >  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> > +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> > @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >bx  lr
> >
> >
> > +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> > +  mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
> > +  bx  lr
> > +
> > +
> >  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
> >mcr p15, 0, r0, c7, c14, 1  @clean and invalidate single data cache 
> > line
> >bx  lr
> > diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm 
> > b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> > index df7e22dca2d9..a460bd2da7a9 100644
> > --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> > +++

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-22 Thread Laszlo Ersek 
On 01/22/19 16:09, Laszlo Ersek wrote:

> (I plan to undo commit cf580da1bc4c ("ArmPkg/ArmLib: don't invalidate
> entire I-cache on range operation", 2016-05-12) and commit b7de7e3cab3f
> ("ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU",
> 2015-12-08), and to ask the reporter to reproduce the issue like that,
> but I figured I'd ask first.)

OK, I have to apologize, my lack of knowledge of aarch64 shows here. It
seems that the icache can only be invalidated to PoU (the ARM ARM only
lists IC IALLU / IALLUIS / IVAU), so undoing cf580da1bc4c should be useless.

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2019-01-22 Thread Laszlo Ersek 
Performing thread-necromancy on a patch from 2015, which is today known
as commit b7de7e3cab3f. Also CC'ing Christoffer and Marc.

Question at the bottom.

On 12/07/15 08:06, Ard Biesheuvel wrote:
> From: "Cohen, Eugene" 
>
> This patch updates the ArmPkg variant of InvalidateInstructionCacheRange to
> flush the data cache only to the point of unification (PoU). This improves
> performance and also allows invalidation in scenarios where it would be
> inappropriate to flush to the point of coherency (like when executing code
> from L2 configured as cache-as-ram).
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Eugene Cohen 
>
> Added AARCH64 and ARM/GCC implementations of the above.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Include/Library/ArmLib.h| 8 +++-
>  ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c | 2 +-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6 ++
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6 ++
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5 +
>  5 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 9622444ec63f..85fa1f600ba9 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
>
>  VOID
>  EFIAPI
> -ArmCleanDataCacheEntryByMVA (
> +ArmCleanDataCacheEntryToPoUByMVA(
>IN  UINTN   Address
>);
>
>  VOID
>  EFIAPI
> +ArmCleanDataCacheEntryByMVA(
> +IN  UINTN   Address
> +);
> +
> +VOID
> +EFIAPI
>  ArmCleanInvalidateDataCacheEntryByMVA (
>IN  UINTN   Address
>);
> diff --git a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c 
> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> index feab4497ac1b..1045f9068f4d 100644
> --- a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> +++ b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.c
> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryByMVA);
> +  CacheRangeOperation (Address, Length, ArmCleanDataCacheEntryToPoUByMVA);
>ArmInvalidateInstructionCache ();
>return Address;
>  }
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index c530d19e897e..db21f73f0ed7 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -22,6 +22,7 @@
>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>ret
>
>
> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> +  dc  cvau, x0// Clean single data cache line to PoU
> +  ret
> +
> +
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>dc  civac, x0   // Clean and invalidate single data cache line
>ret
> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S 
> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> index 5f030d92de31..7de1b11ef818 100644
> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> @@ -19,6 +19,7 @@
>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>bx  lr
>
>
> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> +  mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
> +  bx  lr
> +
> +
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>mcr p15, 0, r0, c7, c14, 1  @clean and invalidate single data cache 
> line
>bx  lr
> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm 
> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> index df7e22dca2d9..a460bd2da7a9 100644
> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> @@ -34,6 +34,11 @@ CTRL_I_BIT  EQU (1 << 12)
>bx  lr
>
>
> + RVCT_ASM_EXPORT ArmCleanDataCacheEntryToPoUByMVA
> +  mcr p15, 0, r0, c7, c11, 1  ; clean single data cache line to PoU
> +  bx  lr
> +
> +
>   RVCT_ASM_EXPORT

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2015-12-08 Thread Cohen, Eugene 
Reviewed-by: Eugene Cohen 

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Monday, December 07, 2015 12:06 AM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Cohen, Eugene
> 
> Cc: Ard Biesheuvel 
> Subject: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to
> flush only to PoU
> 
> From: "Cohen, Eugene" 
> 
> This patch updates the ArmPkg variant of
> InvalidateInstructionCacheRange to
> flush the data cache only to the point of unification (PoU). This
> improves
> performance and also allows invalidation in scenarios where it would
> be
> inappropriate to flush to the point of coherency (like when executing
> code
> from L2 configured as cache-as-ram).
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Eugene Cohen 
> 
> Added AARCH64 and ARM/GCC implementations of the above.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Include/Library/ArmLib.h| 8 +++-
> 
> ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib
> .c | 2 +-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6
> ++
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6
> ++
>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5
> +
>  5 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h
> b/ArmPkg/Include/Library/ArmLib.h
> index 9622444ec63f..85fa1f600ba9 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
> 
>  VOID
>  EFIAPI
> -ArmCleanDataCacheEntryByMVA (
> +ArmCleanDataCacheEntryToPoUByMVA(
>IN  UINTN   Address
>);
> 
>  VOID
>  EFIAPI
> +ArmCleanDataCacheEntryByMVA(
> +IN  UINTN   Address
> +);
> +
> +VOID
> +EFIAPI
>  ArmCleanInvalidateDataCacheEntryByMVA (
>IN  UINTN   Address
>);
> diff --git
> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> ib.c
> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> ib.c
> index feab4497ac1b..1045f9068f4d 100644
> ---
> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> ib.c
> +++
> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> ib.c
> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
>IN  UINTN Length
>)
>  {
> -  CacheRangeOperation (Address, Length,
> ArmCleanDataCacheEntryByMVA);
> +  CacheRangeOperation (Address, Length,
> ArmCleanDataCacheEntryToPoUByMVA);
>ArmInvalidateInstructionCache ();
>return Address;
>  }
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index c530d19e897e..db21f73f0ed7 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -22,6 +22,7 @@
>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>ret
> 
> 
> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> +  dc  cvau, x0// Clean single data cache line to PoU
> +  ret
> +
> +
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>dc  civac, x0   // Clean and invalidate single data cache line
>ret
> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> index 5f030d92de31..7de1b11ef818 100644
> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> @@ -19,6 +19,7 @@
>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>bx  lr
> 
> 
> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> +  mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
> +  bx  lr
> +
> +
>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>mcr p15, 0, r0, c7, c14, 1  @clean and invalidate single data cache
> line
>bx  lr
> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
> index df7e22dca2d9..a460bd2da7a9 100644
> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm
>

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2015-12-08 Thread Ard Biesheuvel 
On 8 December 2015 at 14:05, Cohen, Eugene  wrote:
> Reviewed-by: Eugene Cohen 
>

Thanks Eugene

@Leif: any concerns?


>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Monday, December 07, 2015 12:06 AM
>> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Cohen, Eugene
>> 
>> Cc: Ard Biesheuvel 
>> Subject: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to
>> flush only to PoU
>>
>> From: "Cohen, Eugene" 
>>
>> This patch updates the ArmPkg variant of
>> InvalidateInstructionCacheRange to
>> flush the data cache only to the point of unification (PoU). This
>> improves
>> performance and also allows invalidation in scenarios where it would
>> be
>> inappropriate to flush to the point of coherency (like when executing
>> code
>> from L2 configured as cache-as-ram).
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Eugene Cohen 
>>
>> Added AARCH64 and ARM/GCC implementations of the above.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Include/Library/ArmLib.h| 8 +++-
>>
>> ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib
>> .c | 2 +-
>>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6
>> ++
>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6
>> ++
>>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5
>> +
>>  5 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPkg/Include/Library/ArmLib.h
>> b/ArmPkg/Include/Library/ArmLib.h
>> index 9622444ec63f..85fa1f600ba9 100644
>> --- a/ArmPkg/Include/Library/ArmLib.h
>> +++ b/ArmPkg/Include/Library/ArmLib.h
>> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
>>
>>  VOID
>>  EFIAPI
>> -ArmCleanDataCacheEntryByMVA (
>> +ArmCleanDataCacheEntryToPoUByMVA(
>>IN  UINTN   Address
>>);
>>
>>  VOID
>>  EFIAPI
>> +ArmCleanDataCacheEntryByMVA(
>> +IN  UINTN   Address
>> +);
>> +
>> +VOID
>> +EFIAPI
>>  ArmCleanInvalidateDataCacheEntryByMVA (
>>IN  UINTN   Address
>>);
>> diff --git
>> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> ib.c
>> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> ib.c
>> index feab4497ac1b..1045f9068f4d 100644
>> ---
>> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> ib.c
>> +++
>> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> ib.c
>> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
>>IN  UINTN Length
>>)
>>  {
>> -  CacheRangeOperation (Address, Length,
>> ArmCleanDataCacheEntryByMVA);
>> +  CacheRangeOperation (Address, Length,
>> ArmCleanDataCacheEntryToPoUByMVA);
>>ArmInvalidateInstructionCache ();
>>return Address;
>>  }
>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> index c530d19e897e..db21f73f0ed7 100644
>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> @@ -22,6 +22,7 @@
>>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
>> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>>ret
>>
>>
>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>> +  dc  cvau, x0// Clean single data cache line to PoU
>> +  ret
>> +
>> +
>>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>>dc  civac, x0   // Clean and invalidate single data cache line
>>ret
>> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> index 5f030d92de31..7de1b11ef818 100644
>> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> @@ -19,6 +19,7 @@
>>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
>> @@ -69,6 +70,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>>bx  lr
>>
>>
>> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>> +  mcr p15, 0, r0, c7, c11, 1  @clean single data cache line to PoU
>> +  bx  lr
>> +
>> +
>>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>>mcr p15, 0, r0, c7, c14, 1  @clean and invalidate single data

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2015-12-08 Thread Leif Lindholm 
On Tue, Dec 08, 2015 at 04:19:47PM +0100, Ard Biesheuvel wrote:
> On 8 December 2015 at 14:05, Cohen, Eugene  wrote:
> > Reviewed-by: Eugene Cohen 
> >
> 
> Thanks Eugene
> 
> @Leif: any concerns?

None - just stuck in a meeting :)
Thanks.

Reviewed-by: Leif Lindholm 

> >> -Original Message-
> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >> Sent: Monday, December 07, 2015 12:06 AM
> >> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Cohen, Eugene
> >> 
> >> Cc: Ard Biesheuvel 
> >> Subject: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to
> >> flush only to PoU
> >>
> >> From: "Cohen, Eugene" 
> >>
> >> This patch updates the ArmPkg variant of
> >> InvalidateInstructionCacheRange to
> >> flush the data cache only to the point of unification (PoU). This
> >> improves
> >> performance and also allows invalidation in scenarios where it would
> >> be
> >> inappropriate to flush to the point of coherency (like when executing
> >> code
> >> from L2 configured as cache-as-ram).
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Eugene Cohen 
> >>
> >> Added AARCH64 and ARM/GCC implementations of the above.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>  ArmPkg/Include/Library/ArmLib.h| 8 
> >> +++-
> >>
> >> ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib
> >> .c | 2 +-
> >>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6
> >> ++
> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6
> >> ++
> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5
> >> +
> >>  5 files changed, 25 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/ArmPkg/Include/Library/ArmLib.h
> >> b/ArmPkg/Include/Library/ArmLib.h
> >> index 9622444ec63f..85fa1f600ba9 100644
> >> --- a/ArmPkg/Include/Library/ArmLib.h
> >> +++ b/ArmPkg/Include/Library/ArmLib.h
> >> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
> >>
> >>  VOID
> >>  EFIAPI
> >> -ArmCleanDataCacheEntryByMVA (
> >> +ArmCleanDataCacheEntryToPoUByMVA(
> >>IN  UINTN   Address
> >>);
> >>
> >>  VOID
> >>  EFIAPI
> >> +ArmCleanDataCacheEntryByMVA(
> >> +IN  UINTN   Address
> >> +);
> >> +
> >> +VOID
> >> +EFIAPI
> >>  ArmCleanInvalidateDataCacheEntryByMVA (
> >>IN  UINTN   Address
> >>);
> >> diff --git
> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> >> ib.c
> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> >> ib.c
> >> index feab4497ac1b..1045f9068f4d 100644
> >> ---
> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> >> ib.c
> >> +++
> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
> >> ib.c
> >> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
> >>IN  UINTN Length
> >>)
> >>  {
> >> -  CacheRangeOperation (Address, Length,
> >> ArmCleanDataCacheEntryByMVA);
> >> +  CacheRangeOperation (Address, Length,
> >> ArmCleanDataCacheEntryToPoUByMVA);
> >>ArmInvalidateInstructionCache ();
> >>return Address;
> >>  }
> >> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >> index c530d19e897e..db21f73f0ed7 100644
> >> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> >> @@ -22,6 +22,7 @@
> >>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
> >>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
> >> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
> >>ret
> >>
> >>
> >> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
> >> +  dc  cvau, x0// Clean single data cache line to PoU
> >> +  ret
> >> +
> >> +
> >>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
> >>dc  civac, x0   // Clean and invalidate single data cache line
> >>ret
> >> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >> index 5f030d92de31..7de1b11ef818 100644
> >> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
> >> @@ -19,6 +19,7 @@
> >>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
> >>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
> >>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
> >> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
> >>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
> >>  GCC_ASM_EXPORT

Re: [edk2] [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU

2015-12-08 Thread Ard Biesheuvel 
On 8 December 2015 at 16:49, Leif Lindholm  wrote:
> On Tue, Dec 08, 2015 at 04:19:47PM +0100, Ard Biesheuvel wrote:
>> On 8 December 2015 at 14:05, Cohen, Eugene  wrote:
>> > Reviewed-by: Eugene Cohen 
>> >
>>
>> Thanks Eugene
>>
>> @Leif: any concerns?
>
> None - just stuck in a meeting :)
> Thanks.
>
> Reviewed-by: Leif Lindholm 
>

Committed as SVN r19174

Thanks

>> >> -Original Message-
>> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> >> Sent: Monday, December 07, 2015 12:06 AM
>> >> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Cohen, Eugene
>> >> 
>> >> Cc: Ard Biesheuvel 
>> >> Subject: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to
>> >> flush only to PoU
>> >>
>> >> From: "Cohen, Eugene" 
>> >>
>> >> This patch updates the ArmPkg variant of
>> >> InvalidateInstructionCacheRange to
>> >> flush the data cache only to the point of unification (PoU). This
>> >> improves
>> >> performance and also allows invalidation in scenarios where it would
>> >> be
>> >> inappropriate to flush to the point of coherency (like when executing
>> >> code
>> >> from L2 configured as cache-as-ram).
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Eugene Cohen 
>> >>
>> >> Added AARCH64 and ARM/GCC implementations of the above.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ard Biesheuvel 
>> >> ---
>> >>  ArmPkg/Include/Library/ArmLib.h| 8 
>> >> +++-
>> >>
>> >> ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib
>> >> .c | 2 +-
>> >>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 6
>> >> ++
>> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S | 6
>> >> ++
>> >>  ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.asm   | 5
>> >> +
>> >>  5 files changed, 25 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/ArmPkg/Include/Library/ArmLib.h
>> >> b/ArmPkg/Include/Library/ArmLib.h
>> >> index 9622444ec63f..85fa1f600ba9 100644
>> >> --- a/ArmPkg/Include/Library/ArmLib.h
>> >> +++ b/ArmPkg/Include/Library/ArmLib.h
>> >> @@ -183,12 +183,18 @@ ArmInvalidateDataCacheEntryByMVA (
>> >>
>> >>  VOID
>> >>  EFIAPI
>> >> -ArmCleanDataCacheEntryByMVA (
>> >> +ArmCleanDataCacheEntryToPoUByMVA(
>> >>IN  UINTN   Address
>> >>);
>> >>
>> >>  VOID
>> >>  EFIAPI
>> >> +ArmCleanDataCacheEntryByMVA(
>> >> +IN  UINTN   Address
>> >> +);
>> >> +
>> >> +VOID
>> >> +EFIAPI
>> >>  ArmCleanInvalidateDataCacheEntryByMVA (
>> >>IN  UINTN   Address
>> >>);
>> >> diff --git
>> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> >> ib.c
>> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> >> ib.c
>> >> index feab4497ac1b..1045f9068f4d 100644
>> >> ---
>> >> a/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> >> ib.c
>> >> +++
>> >> b/ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceL
>> >> ib.c
>> >> @@ -64,7 +64,7 @@ InvalidateInstructionCacheRange (
>> >>IN  UINTN Length
>> >>)
>> >>  {
>> >> -  CacheRangeOperation (Address, Length,
>> >> ArmCleanDataCacheEntryByMVA);
>> >> +  CacheRangeOperation (Address, Length,
>> >> ArmCleanDataCacheEntryToPoUByMVA);
>> >>ArmInvalidateInstructionCache ();
>> >>return Address;
>> >>  }
>> >> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> >> b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> >> index c530d19e897e..db21f73f0ed7 100644
>> >> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> >> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
>> >> @@ -22,6 +22,7 @@
>> >>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>> >>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)
>> >>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryByMVA)
>> >> +GCC_ASM_EXPORT (ArmCleanDataCacheEntryToPoUByMVA)
>> >>  GCC_ASM_EXPORT (ArmCleanInvalidateDataCacheEntryByMVA)
>> >>  GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryBySetWay)
>> >>  GCC_ASM_EXPORT (ArmCleanDataCacheEntryBySetWay)
>> >> @@ -72,6 +73,11 @@ ASM_PFX(ArmCleanDataCacheEntryByMVA):
>> >>ret
>> >>
>> >>
>> >> +ASM_PFX(ArmCleanDataCacheEntryToPoUByMVA):
>> >> +  dc  cvau, x0// Clean single data cache line to PoU
>> >> +  ret
>> >> +
>> >> +
>> >>  ASM_PFX(ArmCleanInvalidateDataCacheEntryByMVA):
>> >>dc  civac, x0   // Clean and invalidate single data cache line
>> >>ret
>> >> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> >> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> >> index 5f030d92de31..7de1b11ef818 100644
>> >> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> >> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Support.S
>> >> @@ -19,6 +19,7 @@
>> >>  GCC_ASM_EXPORT (ArmInvalidateInstructionCache)
>> >>