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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]


On 4/22/25 5:26 PM, Oleksii Kurochko wrote:
>
>
> 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.

Here I meant flush_dcache() part of copy_to_guest_phys_flush_dcache().

~ Oleksii

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

[-- Attachment #2: Type: text/html, Size: 3419 bytes --]