Re: [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

[Julien Grall <julien.grall@arm.com>]

Hi Shannon,

On 24/03/16 15:06, Shannon Zhao wrote:
> On 2016年03月23日 00:04, Julien Grall wrote:
>> On 22/03/16 13:16, Shannon Zhao wrote:
>>> On 2016年03月22日 00:51, Julien Grall wrote:
>>>>> +        memory_map[offset].Attribute = EFI_MEMORY_WB;
>>>>> +    }
>>>>> +
>>>>> +    for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
>>>>> +    {
>>>>> +        memory_map[offset].Type = EfiACPIReclaimMemory;
>>>>> +        memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
>>>>> +        memory_map[offset].NumberOfPages =
>>>>> PFN_UP(acpi_mem.bank[i].size);
>>>>
>>>> Ditto
>>>>
>>>> You are also assuming that acpi_mem.bank[i].size will always be aligned
>>>> to 4KB. If so, we may expose unwanted data to the guest.
>>>>
>>>> Based on how the field is set, I would add a BUG_ON to ensure this
>>>> condition.
>>> UEFI spec says
>>> "EFI memory descriptors of type EfiACPIReclaimMemory and
>>> EfiACPIMemoryNVS must be aligned on a 4 KiB boundary and must be a
>>> multiple of 4 KiB in size."
>>>
>>> So I think the size is aligned to 4kb, right?
>>
>> Right. I was suggested to add a BUG_ON to document the constraint and
>> ensure nobody will play with acpi_mem outside EFI.
>>
>> A such check would also be nice for mem->bank[i].size;
> sorry, I didn't get the idea. How to add a BUG_ON to check the size?

Something like:

BUG_ON(acpi_mem.bank[i].size & EFI_PAGE_MASK);

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel