Re: [PATCH 09/14] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call

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

Hi Jan,

On 10/05/2019 15:19, Jan Beulich wrote:
>>>> On 10.05.19 at 16:04, <julien.grall@arm.com> wrote:
>> On 10/05/2019 14:45, Jan Beulich wrote:
>>>>>> On 10.05.19 at 15:41, <julien.grall@arm.com> wrote:
>>>> The point here, we keep within the hypervisor the idea of what's valid or
>>>> invalid. This allows us more flexibility on the value here (imagine we decide to
>>>> change the value of GFN_INVALID in the future...).
>>>
>>> Exactly, and hence INVALID_GFN should not become visible to the
>>> outside. Hence my request to use an all-ones value here.
>> It is only visible if you put an exact value in the documentation. Your
>> suggestion is to put a exactly value and I would rather not see that.
> 
> I did specifically suggest to _not_ store INVALID_GFN here, but to
> store 64-bit bits of ones. Note the difference between the two on
> 32-bit Arm.
Your point of having an exact value is only useful if you want to toolstack to 
silently ignore the missing frame and avoid a call.

The former is pretty much wrong as if you were trying to read the frame then 
most likely you wanted to access it. So a message makes sense here.

For the latter, avoiding the call is only going to save you a couple of cycles 
in a likely cold path.

You really don't need to give an exact (including say all ones). You only need 
to say that the address return may not be mappable. The toolstack will try to 
map it and fail. That's not a big deal.

Anyway, I will wait and see what's the view from the tools maintainer.

> 
> Preferably yes; depends on how exactly you code the assertion.
> If you follow the if()-ASSERT_UNREACHABLE()-return style we've
> been using elsewhere, then no matter how you place the #else
> or #endif the printk() will be compiled out.

I will have a look.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel