Re: [PATCH] iommu/vt-d: Check VT-d RMRR region in BIOS is reported as reserved

Re: [PATCH] iommu/vt-d: Check VT-d RMRR region in BIOS is reported as reserved

[Chen, Yian]

On 10/16/2019 12:51 AM, Joerg Roedel wrote:
> Hi,
>
> On Tue, Oct 15, 2019 at 09:49:32AM -0700, Yian Chen wrote:
>> VT-d RMRR (Reserved Memory Region Reporting) regions are reserved
>> for device use only and should not be part of allocable memory pool of OS.
>>
>> BIOS e820_table reports complete memory map to OS, including OS usable
>> memory ranges and BIOS reserved memory ranges etc.
>>
>> x86 BIOS may not be trusted to include RMRR regions as reserved type
>> of memory in its e820 memory map, hence validate every RMRR entry
>> with the e820 memory map to make sure the RMRR regions will not be
>> used by OS for any other purposes.
> Are there real systems in the wild where this is a problem?
Firmware reports e820 and RMRR in separate structure. The system will 
not work stably
if RMRR is not in the e820 table as reserved type mem and can be used 
for other purposes.
In system engineering phase, I practiced with some kind bugs from BIOS, 
but not yet exactly same as
the one here. Please consider this is a generic patch to avoid 
subsequent failure at runtime.
>> +static inline int __init
>> +arch_rmrr_sanity_check(struct acpi_dmar_reserved_memory *rmrr)
>> +{
>> +	u64 start = rmrr->base_address;
>> +	u64 end = rmrr->end_address + 1;
>> +
>> +	if (e820__mapped_all(start, end, E820_TYPE_RESERVED))
>> +		return 0;
>> +
>> +	pr_err(FW_BUG "No firmware reserved region can cover this RMRR [%#018Lx-%#018Lx], contact BIOS vendor for fixes\n",
>> +	       start, end - 1);
>> +	return -EFAULT;
>> +}
> Why -EFAULT, there is no fault involved? Usibg -EINVAL seems to be a better choice.
-EFAULT could be used for address related errors.
For this case, I agree, -EINVAL seems better while
consider it as an input problem from firmware. I will make change.