Re: [Xen-devel] [PATCH v5 09/12] arm/mem_access: Add software guest-page-table walk

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


>
>>
>>> In this case, you need a bit more rationale to explain why setting to
>>> GV2M_READ by default is fine... If it is just a random value, then
>>> say it.
>>>
>>
>> I will add a comment in the change log stating that we set perms to
>> GV2M_READ (that equals to 0) by default to avoid having potentially
>> random values returned on errors.
>
> Beware that you are re-using GV2M_READ for different purpose. In the
> current code it is used to check whether a guest VA is readable.
>
> Here, you are using both GV2M_* to return the permissions of a
> mapping. What matters is whether it is fine to consider a page will
> always have the read permission.
>
> At the moment, you consider you will always translate the VA -> IPA
> with EL1 permission (see D4-29 in ARM DDI 0487B.a). So the resulting
> permission in stage-1 will always contain read.
>
> I am ok if you make this assumption, but this needs to be explained in
> a comment in the code. So we are not surprised in the future why you
> always set read permission.
>

Alright. I will state in a comment in code and change-log that since the
current implementation considers a valid mapping as readable at least by
EL1 because the current implementation does not consider further
attributes responsible for distinguishing between EL0 and EL1. Thank you.

>>
>> Shall I remove your ack for this patch in the next version?
>
> Yes please.
>
> Cheers,
>

Cheers,
~Sergej


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