Re: [PATCH v2 7/8] xen/common: dom0less: introduce common domain-build.c

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/17/25 4:45 PM, Jan Beulich wrote:

> On 14.04.2025 17:56, Oleksii Kurochko wrote:
> > --- a/xen/include/xen/fdt-domain-build.h
> > +++ b/xen/include/xen/fdt-domain-build.h
> > @@ -5,6 +5,7 @@
> >  #include <xen/bootfdt.h>
> >  #include <xen/device_tree.h>
> >  #include <xen/fdt-kernel.h>
> > +#include <xen/mm.h>
> >  #include <xen/types.h>
> >  
> >  #if __has_include(<asm/domain_build.h>)
> > @@ -32,7 +33,37 @@ int make_memory_node(const struct kernel_info *kinfo, int addrcells,
> >                       int sizecells, const struct membanks *mem);
> >  int make_timer_node(const struct kernel_info *kinfo);
> >  
> > -unsigned int get_allocation_size(paddr_t size);
> > +
> > +static inline int get_allocation_size(paddr_t size)
> > +{
> > +    /*
> > +     * get_order_from_bytes returns the order greater than or equal to
> > +     * the given size, but we need less than or equal. Adding one to
> > +     * the size pushes an evenly aligned size into the next order, so
> > +     * we can then unconditionally subtract 1 from the order which is
> > +     * returned.
> > +     */
> > +    return get_order_from_bytes(size + 1) - 1;
> > +}
> > +
> > +typedef unsigned long (*copy_to_guest_phys_cb)(struct domain *d,
> > +                                               paddr_t gpa,
> > +                                               void *buf,
> >       
> This very much looks like the latest now, when the code is made common,
> it wants to be const void *. Even if this may require another prereq
> patch.
>
> However, instead of using a function pointer, couldn't the now common
> code call copy_to_guest_phys_flush_dcache() conditionally upon
> IS_ENABLED(CONFIG_<whatever>)?
>     
I thought about having IS_ENABLED(CONFIG_ARM) as, at the moment, this is necessary to
be called only for Arm as guest domain on Arm could be ran with cache disabled so to be sure
that DTB, kernel and initrd is fully in RAM this function should be called.

For RISC-V, it isn't possible case as guest domain won't run with cache disabled.
> Or provide a weak
> copy_to_guest_phys_flush_dcache() which would simply call
> copy_to_guest_phys()?
>     
Could it be a weak function with empty implementation?
Is copy_to_guest_phys() implemented for other archs?


~ Oleksii