Re: [PATCH v2 07/11] xen: move domain_use_host_layout() to common code

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 3/31/26 5:53 PM, Jan Beulich wrote:
> On 31.03.2026 17:20, Oleksii Kurochko wrote:
> > On 3/30/26 5:13 PM, Jan Beulich wrote:
> > > On 23.03.2026 17:29, Oleksii Kurochko wrote:
> > > > domain_use_host_layout() is not really architecture-specific, so move it
> > > > from the Arm header to the common header xen/domain.h and provide a common
> > > > implementation in xen/common/domain.c. domain_use_host_layout() potentially
> > > > is needed for x86 [1].
> > > No matter that this may indeed be true, ...
> > >
> > > > Turn the macro into a function to avoid header dependency issues.
> > > ... this introduces unreachable code on x86, i.e. a Misra rule 2.1 violation.
> > Do we have some deviation tag for such cases when the code temporary
> > isn't used?
> I'm sorry, but it'll take me about as long as you to find out.
Sure, I will take a look. I just thought that maybe you have a solution 
already just in your head.
 I wonder
> about "temporary" though: Do you have a clear understanding as to when
> that will change?
No, I don't. As Stefano mentioned they will need this function one day. 
Another option we could use ifndef x86 or ifdef DOM0_LESS and then when 
someone will really need it on x86, this ifdef will be dropped. I don't 
know if it is better solution.
It seems like the best one solution will still make a try to make 
declare this function as macro.
> > > > @@ -2544,6 +2544,12 @@ void thaw_domains(void)
> > > >    
> > > >    #endif /* CONFIG_SYSTEM_SUSPEND */
> > > >    
> > > > +bool domain_use_host_layout(struct domain *d)
> > > > +{
> > > > +    return is_domain_direct_mapped(d) ||
> > > > +           (paging_mode_translate(d) && is_hardware_domain(d));
> > > > +}
> > > The placement of paging_mode_translate() doesn't match ...
> > >
> > > > --- a/xen/include/xen/domain.h
> > > > +++ b/xen/include/xen/domain.h
> > > > @@ -62,6 +62,22 @@ void domid_free(domid_t domid);
> > > >    #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
> > > >    #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
> > > >    
> > > > +/*
> > > > + * Is the auto-translated domain using the host memory layout?
> > > > + *
> > > > + * domain_use_host_layout() is always False for PV guests.
> > > ... the description of the function.
> > But why the placement should be different?
> If you focus on auto-translated, then imo paging_mode_translate()
> better would guard everything.
Then it make sense to do in the following way:
 bool domain_use_host_layout(struct domain *d)
 {
-    return is_domain_direct_mapped(d) ||
-           (paging_mode_translate(d) && is_hardware_domain(d));
+    return paging_mode_translate(d) &&
+           (is_domain_direct_mapped(d) || is_hardware_domain(d));
 }

> > is_domain_direct_mapped() is false for PV guests (and for other guest
> > types on x86).
> >
> > So if domain_use_host_layout() is fully depends on
> > paging_mode_translate(d) && is_hardware_domain(d) and for which
> > paging_mode_translate() is false if it is PV guest.
> > Thereby domain_use_host_layout() is false too.
> >
> > > Further, the first sentence above suggests the caller has to check
> > > paging_mode_translate() before calling, which as per the implementation
> > > clearly isn't the intention.
> > Sorry, I don't follow you here.
> By starting the comment with "Is the auto-translated domain using", you
> imply the caller checked for that aspect already. At least the way I
> read it.
My understanding was that it is an explanation what function is checking.

~ Oleksii