Re: [PATCH] xen: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.02.2021 21:51, Stefano Stabellini wrote:
> On Thu, 25 Feb 2021, Jan Beulich wrote:
>> On 25.02.2021 02:22, Stefano Stabellini wrote:
>>> --- a/xen/include/public/features.h
>>> +++ b/xen/include/public/features.h
>>> @@ -114,6 +114,13 @@
>>>   */
>>>  #define XENFEAT_linux_rsdp_unrestricted   15
>>>  
>>> +/*
>>> + * A direct-mapped (or 1:1 mapped) domain is a domain for which its
>>> + * local pages have gfn == mfn.
>>> + */
>>> +#define XENFEAT_not_direct_mapped       16
>>> +#define XENFEAT_direct_mapped           17
>>
>> Why two new values? Absence of XENFEAT_direct_mapped requires
>> implying not-direct-mapped by the consumer anyway, doesn't it?
> 
> That's because if we add both flags we can avoid all unpleasant guessing
> games in the guest kernel.
> 
> If one flag or the other flag is set, we can make an informed decision.
> 
> But if neither flag is set, it means we are running on an older Xen,
> and we fall back on the current checks.

Oh, okay - if there's guesswork to avoid, then I see the point.
Maybe mention in the description?

>> Further, quoting xen/mm.h: "For a non-translated guest which
>> is aware of Xen, gfn == mfn." This to me implies that PV would
>> need to get XENFEAT_direct_mapped set; not sure whether this
>> simply means x86'es is_domain_direct_mapped() is wrong, but if
>> it is, uses elsewhere in the code would likely need changing.
> 
> That's a good point, I didn't think about x86 PV. I think the two flags
> are needed for autotranslated guests. I don't know for sure what is best
> for non-autotranslated guests.
> 
> Maybe we could say that XENFEAT_not_direct_mapped and
> XENFEAT_direct_mapped only apply to XENFEAT_auto_translated_physmap
> guests. And it would match the implementation of
> is_domain_direct_mapped().

I'm having trouble understanding this last sentence, and hence I'm
not sure I understand the rest in the way you may mean it. Neither
x86'es nor Arm's is_domain_direct_mapped() has any check towards a
guest being translated (obviously such a check would be redundant
on Arm).

> For non XENFEAT_auto_translated_physmap guests we could either do:
> 
> - neither flag is set
> - set XENFEAT_direct_mapped (without changing the implementation of
>   is_domain_direct_mapped)
> 
> What do you think? I am happy either way.

I'm happy either way as well; suitably described perhaps setting
XENFEAT_direct_mapped when !paging_mode_translate() would be
slightly more "natural". But a spelled out and enforced
dependency upon XENFEAT_auto_translated_physmap would too be fine
with me.

Jan