Re: [Xen-devel] [PATCH v2 1/2] arm/mem_access: adjust check_and_get_page to not rely on current

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Mon, Dec 12, 2016 at 2:28 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Tamas,
>
>
> On 12/12/2016 19:41, Tamas K Lengyel wrote:
>>
>> On Mon, Dec 12, 2016 at 12:11 PM, Julien Grall <julien.grall@xxxxxxx>
>> wrote:
>>>
>>> Hi Tamas,
>>>
>>> On 12/12/16 18:42, Tamas K Lengyel wrote:
>>>>
>>>>
>>>> On Mon, Dec 12, 2016 at 4:46 AM, Julien Grall <julien.grall@xxxxxxx>
>>>> wrote:
>>>>>
>>>>>
>>>>> The translation VA to IPA (guest physical address) is done using
>>>>> hardware.
>>>>> If the underlying memory of the stage-1 page table is protected, so the
>>>>> translation will fail. Given that this function is used in hypercall to
>>>>> retrieve the page associated to a buffer, it means that it will not be
>>>>> possible to do hypercall when the page table used to find the buffer
>>>>> IPA
>>>>> has
>>>>> not been touched.
>>>>
>>>>
>>>>
>>>> This function specifically works around the case where the page of the
>>>> guest pagetable is not accessible due to mem_access, when the hardware
>>>> based lookup doesn't work.This function checks what the fault was,
>>>> checks the page type and the mem_access rights to determine whether
>>>> the fault was legit, or if it was due to mem_access. If it was
>>>> mem_access it gets the page without involving the hardware. I'm not
>>>> following what you describe afterwards regarding the buffer and what
>>>> you mean by "the buffer IPA has not been touched". Care to elaborate?
>>>
>>>
>>>
>>> I am afraid to say that the function does not do what you think and is
>>> still
>>> using the hardware to do the translation. For instance the function
>>> gva_to_ipa is using the hardware to translate a VA to IPA.
>>>
>>> This function is called when it is not possible to directly translate a
>>> VA
>>> to a PA. This may fail for various reason:
>>>         * The underlying memory of the buffer was restricted in stage-2
>>>         * The underlying memory of stage-1 page tables was restricted in
>>> stage-2
>>>
>>> Whilst the function is solving the former, the latter will not work due
>>> to
>>> the call to gva_to_ipa. This will fail because the stage-1 PT are not
>>> accessible.
>>
>>
>> I see. So IMHO this is not a problem with mem_access in general, but a
>> problem with a specific application of mem_access on ARM (ie.
>> restricting read access to guest pagetables). It's a pitty that ARM
>> doesn't report the IPA automatically during a stage-2 violation.
>
>
> I don't understand what you are asking for here. If you are not able to
> access stage-1 page table how would you be able to find the IPA?

I'm not asking for anything, I'm expressing how it's a pity that ARM
CPUs are limited in this regard compared to x86.

>
> It works on x86 because, IIRC, you do a software page table walking.
> Although, I don't think you have particular write/read access checking on
> x86.

I don't recall there being any software page walking being involved on
x86. Why would that be needed? On x86 we get the guest physical
address recorded by the CPU automatically. AFAIK in case the pagetable
was unaccessible for the translation of a VA, we would get an event
with the pagetable's PA and the type of event that generated it (ie.
reading during translation).

>
>> A way to work around this would require mem_access restrictions to be
>> complete removed, which cannot be done unless all other vCPUs of the
>> domain are paused to avoid a race-condition. With altp2m I could also
>> envision creating a temporary p2m for the vcpu at hand with the
>> restriction removed, so that it doesn't affect other vcpus. However,
>> without a use-case specifically requiring this to be implemented I
>> would not deem it critical.
>
>
> I suggested a use-case on the previous e-mail... You are affected today
> because Linux is creating hypercall buffer on the stack and heap. So the
> memory would always be accessed before. I could foresee guest using const
> hypercall buffer.
>
>> For now a comment in the header describing
>> this limitation would suffice from my perspective.
>
>
> So you are going to defer everything until someone actually hit it? It might
> be time for you to focus a bit more on other use case...
>

Yes, as long as this is not a critical issue that breaks mem_access
and can be worked around I'll postpone spending time on it. If someone
finds the time in the meanwhile to submit a patch fixing it I would be
happy to review and test it.

Tamas

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