Re: [PATCH V5 3/4] arm64: apei: implement arch_apei_get_mem_attributes()

["Zhang, Jonathan Zhixiong" <zjzhang@codeaurora.org>]

Thanks for the feedback, Hanjun.

On 7/17/2015 3:06 AM, Hanjun Guo wrote:
> On 07/17/2015 09:37 AM, Zhang, Jonathan Zhixiong wrote:
>> Thank you Will for the feedback. Pls. see comments below.
>>
>> On 7/16/2015 10:18 AM, Will Deacon wrote:
>>> On Wed, Jul 15, 2015 at 01:31:55AM +0100, Jonathan (Zhixiong) Zhang
>>> wrote:
>>>> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>>>>
>>>> If the physical address has memory attributes defined by EFI
>>>> memmap as EFI_MEMORY_UC, the page protection type is
>>>> PROT_DEVICE_nGnRE. Otherwise, the page protection type is
>>>> PAGE_KERNEL.
>>>>
>>>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>>>> ---
>>>>   arch/arm64/kernel/Makefile |  1 +
>>>>   arch/arm64/kernel/apei.c   | 27 +++++++++++++++++++++++++++
>>>>   2 files changed, 28 insertions(+)
>>>>   create mode 100644 arch/arm64/kernel/apei.c
>>>>
>>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>>> index 426d0763c81b..c135f1e4e3b8 100644
>>>> --- a/arch/arm64/kernel/Makefile
>>>> +++ b/arch/arm64/kernel/Makefile
>>>> @@ -36,6 +36,7 @@ arm64-obj-$(CONFIG_EFI)            += efi.o
>>>> efi-stub.o efi-entry.o
>>>>   arm64-obj-$(CONFIG_PCI)            += pci.o
>>>>   arm64-obj-$(CONFIG_ARMV8_DEPRECATED)    += armv8_deprecated.o
>>>>   arm64-obj-$(CONFIG_ACPI)        += acpi.o
>>>> +arm64-obj-$(CONFIG_ACPI_APEI)        += apei.o
>>>>
>>>>   obj-y                    += $(arm64-obj-y) vdso/
>>>>   obj-m                    += $(arm64-obj-m)
>>>> diff --git a/arch/arm64/kernel/apei.c b/arch/arm64/kernel/apei.c
>>>> new file mode 100644
>>>> index 000000000000..9c6b62f40ad7
>>>> --- /dev/null
>>>> +++ b/arch/arm64/kernel/apei.c
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Arch-specific APEI-related functions.
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> modify
>>>> + * it under the terms of the GNU General Public License as
>>>> published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> + * GNU General Public License for more details.
>>>> + */
>>>> +
>>>> +#include <acpi/apei.h>
>>>> +
>>>> +#include <linux/efi.h>
>>>> +
>>>> +#include <asm/pgtable.h>
>>>> +
>>>> +pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>>>> +{
>>>> +    if (efi_mem_attributes(addr) & EFI_MEMORY_UC)
>>>> +        return PROT_DEVICE_nGnRE;
>>>> +    else
>>>> +        return PAGE_KERNEL;
>>>> +}
>>>
>>> Do we really need a new file and out-of-line call for this?
>> We have a choice of either adding this function to
>> arch/arm64/kernel/acpi.c, or creating
>> arch/arm64/kernel/apei.c. As we continue to work on firmware first
>> HW error handling for arm64, more arm64 specific APEI related functions
>> may need to be implemented, thus I think it would be good to create
>> arch/arm64/kernel/apei.c. That being said, to date we have found
>> the needs to have only two arm64 specific APEI related functions.
>> The other one can be found in LEG kernel, through this commit:
>>    aa2d69c88b27 ACPI, APEI, ARM64: APEI initial support for aarch64
>> My understanding is that Linaro will work on to upstream that commit. I
>> do not strongly prefer either choice.
>
> For me, I would like to locate those codes in arm64/kernel/acpi.c and
> guard with CONFIG_ACPI_APEI now, if we want to extend in the future,
> then let's see if we do anything for that, in this way, we can make ACPI
> on ARM64 (the arch code) self-contained.
Makes sense. Will do.
>
> Thanks
> Hanjun

-- 
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project