Re: [PATCH] xen: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 26 Feb 2021, Jan Beulich wrote:
> 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?

Yes, I can do that.


> >> 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).

I meant that we are not explicitely checking for auto_translated in
neither version of is_domain_direct_mapped(), but it is sort of implied.


> > 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.

OK. I'll go with:

            if ( is_domain_direct_mapped(d) || !paging_mode_translate(d) )
                fi.submap |= (1U << XENFEAT_direct_mapped);
            else
                fi.submap |= (1U << XENFEAT_not_direct_mapped);


With an appropriate explanation.