* [PATCH] ACPI/APEI: correct/extend pgprot_t retrieval to map GHES memory
@ 2015-12-22 15:50 Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2015-12-22 15:50 UTC (permalink / raw)
To: linux-acpi; +Cc: zjzhang, Ingo Molnar, Matt Fleming
Commit 8ece249a81 ("acpi/apei: Use appropriate pgprot_t to map GHES
memory") adjusted ghes_ioremap_pfn_irq() but left ghes_ioremap_pfn_nmi()
alone, and while doing the adjustment it introduced an at least latent
truncation issue on ix86 (using "unsigned long" for a physical address).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Cc: Matt Fleming <mfleming@suse.com>
Cc: Ingo Molnar <mingo@kernel.org>
---
drivers/acpi/apei/ghes.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
--- 4.4-rc6/drivers/acpi/apei/ghes.c
+++ 4.4-rc6-ACPI-GHES-ioremap/drivers/acpi/apei/ghes.c
@@ -147,17 +147,23 @@ static void ghes_ioremap_exit(void)
static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
{
unsigned long vaddr;
+ phys_addr_t paddr;
+ pgprot_t prot;
vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
- ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
- pfn << PAGE_SHIFT, PAGE_KERNEL);
+
+ paddr = pfn << PAGE_SHIFT;
+ prot = arch_apei_get_mem_attribute(paddr);
+
+ ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
return (void __iomem *)vaddr;
}
static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
{
- unsigned long vaddr, paddr;
+ unsigned long vaddr;
+ phys_addr_t paddr;
pgprot_t prot;
vaddr = (unsigned long)GHES_IOREMAP_IRQ_PAGE(ghes_ioremap_area->addr);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ACPI/APEI: correct/extend pgprot_t retrieval to map GHES memory
2015-12-29 12:28 ` Matt Fleming
@ 2016-01-06 8:43 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2016-01-06 8:43 UTC (permalink / raw)
To: Matt Fleming
Cc: Borislav Petkov, zjzhang, Ingo Molnar, Rafael J. Wysocki, linux-acpi
>>> On 29.12.15 at 13:28, <mfleming@suse.com> wrote:
> On Tue, 22 Dec, at 03:50:45PM, Jan Beulich wrote:
>> Commit 8ece249a81 ("acpi/apei: Use appropriate pgprot_t to map GHES
>> memory") adjusted ghes_ioremap_pfn_irq() but left ghes_ioremap_pfn_nmi()
>> alone, and while doing the adjustment it introduced an at least latent
>> truncation issue on ix86 (using "unsigned long" for a physical address).
>
> Good catch, but I don't think this is the correct fix. 'paddr' should
> be u64 because that's the data type used in ghes_copy_tofrom_phys(),
> and because the value is read from ACPI. The firmware dictates the
> data types.
Making paddr u64 won't be of any use when phys_addr_t is only
32 bits, since passing the value to arch_apei_get_mem_attribute()
and ioremap_page_range() would still cause truncation.
>> --- 4.4-rc6/drivers/acpi/apei/ghes.c
>> +++ 4.4-rc6-ACPI-GHES-ioremap/drivers/acpi/apei/ghes.c
>> @@ -147,17 +147,23 @@ static void ghes_ioremap_exit(void)
>> static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
>> {
>> unsigned long vaddr;
>> + phys_addr_t paddr;
>> + pgprot_t prot;
>>
>> vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
>> - ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
>> - pfn << PAGE_SHIFT, PAGE_KERNEL);
>> +
>> + paddr = pfn << PAGE_SHIFT;
>> + prot = arch_apei_get_mem_attribute(paddr);
>> +
>> + ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
>>
>> return (void __iomem *)vaddr;
>> }
>
> Did you test this change? I didn't suggest fixing this function up
> originally because it isn't used on arm64.
Do you see anything wrong here? I can't see the change do
anything that could break things. And the function - used there
or not - is being compiled for ARM64, and even from an
abstract perspective it seems wrong to use
arch_apei_get_mem_attribute() in its non-NMI counterpart but
not here.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ACPI/APEI: correct/extend pgprot_t retrieval to map GHES memory
[not found] <56797F6502000078000C2525@suse.com>
@ 2015-12-29 12:28 ` Matt Fleming
2016-01-06 8:43 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2015-12-29 12:28 UTC (permalink / raw)
To: Jan Beulich
Cc: linux-acpi, zjzhang, Ingo Molnar, Rafael J. Wysocki, Borislav Petkov
On Tue, 22 Dec, at 03:50:45PM, Jan Beulich wrote:
> Commit 8ece249a81 ("acpi/apei: Use appropriate pgprot_t to map GHES
> memory") adjusted ghes_ioremap_pfn_irq() but left ghes_ioremap_pfn_nmi()
> alone, and while doing the adjustment it introduced an at least latent
> truncation issue on ix86 (using "unsigned long" for a physical address).
Good catch, but I don't think this is the correct fix. 'paddr' should
be u64 because that's the data type used in ghes_copy_tofrom_phys(),
and because the value is read from ACPI. The firmware dictates the
data types.
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Cc: Matt Fleming <mfleming@suse.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> ---
> drivers/acpi/apei/ghes.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> --- 4.4-rc6/drivers/acpi/apei/ghes.c
> +++ 4.4-rc6-ACPI-GHES-ioremap/drivers/acpi/apei/ghes.c
> @@ -147,17 +147,23 @@ static void ghes_ioremap_exit(void)
> static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
> {
> unsigned long vaddr;
> + phys_addr_t paddr;
> + pgprot_t prot;
>
> vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
> - ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
> - pfn << PAGE_SHIFT, PAGE_KERNEL);
> +
> + paddr = pfn << PAGE_SHIFT;
> + prot = arch_apei_get_mem_attribute(paddr);
> +
> + ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
>
> return (void __iomem *)vaddr;
> }
Did you test this change? I didn't suggest fixing this function up
originally because it isn't used on arm64.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-06 8:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-22 15:50 [PATCH] ACPI/APEI: correct/extend pgprot_t retrieval to map GHES memory Jan Beulich
[not found] <56797F6502000078000C2525@suse.com>
2015-12-29 12:28 ` Matt Fleming
2016-01-06 8:43 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.