Re: [PATCH v2 32/41] arm : acpi dynamically map mmio regions

[Parth Dixit <parth.dixit@linaro.org>]

+shannon

On 15 June 2015 at 06:49, Julien Grall <julien.grall@citrix.com> wrote:
> Hi Parth,
>
> On 14/06/2015 11:27, Parth Dixit wrote:
>>
>> On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote:
>>>
>>> Hi Parth,
>>>
>>> On 17/05/2015 21:03, Parth Dixit wrote:
>>>>
>>>>
>>>> In ACPI mmio regions are described in DSDT which requires
>>>> AML interpreter. Defer the mapping of mmio until DSDT is parsed in
>>>> DOM0 and map mmio's dynamically at the time of request.
>>>
>>>
>>>
>>> I'm against a such solution. Even though it's DOM0 we should avoid to
>>> allow
>>> him to map anything (RAM,...) on data abort.
>>
>> I think we agreed to this solution
>> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html
>
>
> Firstly, this kind of link should have been put in the changelog of the
> patch (after ---). It helps the reviewer to know what was decided (or not)
> on the previous discussion. It's more true with a series of more than 40
> patches...
>
> Secondly, the thread you pointed as some discussion on it but no formal
> agreement about what to do. If there is no answer, it's better to do a
> resume and ask if anyone are agree.
>
> Finally, what you've implemented and what was suggested by Ian is different.
> You are allowing any region to be mapped in DOM0 via this way. Only MMIO
> should be allowed.
>
> Concerning the mapping attribute used. Based on Ard answer "The UEFI spec
> mandates that the memory map describes all memory in the system, so if dom0
> accesses any ranges outside of that, it makes sense
> to just use device mappings for stage 2.". We should use by default Device
> Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be
> memory, which if I wasn't able to prove), then we can think differently.
>
> For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and
> any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does
> that). It would keep the ACPI code contained at boot time and no difference
> during runtime.
>
> Regards,
>
> --
> Julien Grall