Thank you Borislav for the review. Pls. see comments inline...
On 6/5/2015 3:25 AM, Borislav Petkov wrote:
On Fri, Jun 05, 2015 at 10:57:01AM +0100, Matt Fleming wrote:
[ Cc'ing Boris and Tony. Folks original patch is here, https://lkml.kernel.org/r/1433185940-24770-4-git-send-email-zjzhang@codeauro... ]
On Mon, 01 Jun, at 12:12:20PM, Jonathan (Zhixiong) Zhang wrote:
From: "Jonathan (Zhixiong) Zhang" zjzhang@codeaurora.org
With ACPI APEI firmware first handling, generic hardware error record is updated by firmware in GHES memory region. When firmware updated GHES memory region in DDR without going through cache,
What is DDR?
I think this needs to be clarified first before we go any further.
I thought the word "memory" might be confusing, because there are memories on the system that is not accessible by Linux. In this context, the APEI error data is accessed (read and write) by both Linux and platform firmware; hence both sides should access the memory using same cache attribute. I wanted to emphasize the idea that even though normally DDR is cachable, but in this case when platform access it with un-cached attribute, Linux should do the same. I will try to make it more clear in next version of the patch.
I picked up on the sidelines that this might be arm64-specific stuff. If so, your approach is wrong: you're merging efi_* facilities from x86 and ia64 into generic efi ones but then doing CONFIG_EFI ifdeffery in GHES.
What you should do instead is have arch-specific:
ghes_ioremap_pfn_irq() ghes_iounmap_irq() ...
and whatever else functionality which is different on your arch and which get called from the generic ghes.c driver.
In the arch-specific ones you can go wild with the ifdeffery and whatnot is needed on that specific arch.
Something like that, at least.
Makes total sense. I was trying to reduce binary size for non-EFI system, but as Matt pointed out in another feedback, on x86 even BIOS based system has CONFIG_EFI enabled. I will submit a new version accordingly.