Re: [PATCH v2 3/5] arm/dom0less: put dom0less feature code in a separate module

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Julien,

>> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
>> new file mode 100644
>> index 000000000000..dc9c90cf00a7
>> --- /dev/null
>> +++ b/xen/arch/arm/dom0less-build.c
>> @@ -0,0 +1,1087 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * xen/arch/arm/dom0less-build.c
>> + *
>> + * Code related to the dom0less functionality
>> + *
>> + * Copyright (C) 2023 Arm Ltd.
> 
> This feels a bit odd to add your copyright here. It sounds like Arm wrote all 
> the code, but you only moved. That said, I am not a lawyer.

Mmm yes, we should be required to put our copyright in everything we create or 
modify or touch,
said that, I could modify that to be like this:

/*
 * xen/arch/arm/dom0less-build.c 
 *
 * Code related to the dom0less functionality
 *
 * Original code from xen/arch/arm/domain_build.c
 * Modifications: Copyright (C) 2023 Arm Ltd.
 */

What do you think?

> 
>> + */
>> +
>> +#include <xen/device_tree.h>
>> +#include <xen/err.h>
>> +#include <xen/event.h>
>> +#include <xen/grant_table.h>
>> +#include <xen/iocap.h>
>> +#include <xen/libfdt/libfdt.h>
>> +#include <xen/sched.h>
>> +#include <xen/serial.h>
>> +#include <xen/sizes.h>
>> +#include <xen/vmap.h>
>> +
>> +#include <asm/arm64/sve.h>
>> +#include <asm/dom0less-build.h>
>> +#include <asm/domain_build.h>
>> +
>> +bool __init is_dom0less_mode(void)
>> +{
>> +    struct bootmodules *mods = &bootinfo.modules;
>> +    struct bootmodule *mod;
>> +    unsigned int i;
>> +    bool dom0found = false;
>> +    bool domUfound = false;
>> +
>> +    /* Look into the bootmodules */
>> +    for ( i = 0 ; i < mods->nr_mods ; i++ )
>> +    {
>> +        mod = &mods->module[i];
>> +        /* Find if dom0 and domU kernels are present */
>> +        if ( mod->kind == BOOTMOD_KERNEL )
>> +        {
>> +            if ( mod->domU == false )
>> +            {
>> +                dom0found = true;
>> +                break;
>> +            }
>> +            else
>> +                domUfound = true;
>> +        }
>> +    }
>> +
>> +    /*
>> +     * If there is no dom0 kernel but at least one domU, then we are in
>> +     * dom0less mode
>> +     */
>> +    return ( !dom0found && domUfound );
>> +}
>> +
>> +static bool __init allocate_bank_memory(struct domain *d,
>> +                                        struct kernel_info *kinfo,
>> +                                        gfn_t sgfn,
>> +                                        paddr_t tot_size)
> 
> I understand that this is today only used by domUs. However, we could 
> technically use it for allocating dom0 memory if it is not 1:1.
> 
> So I think this function should stay in domain_build.c.

Ok I can leave it there, is it ok for you if I protect it using the KConfig in 
domain_build.c since that code is currently not used apart from dom0less?
(I will do that in the last patch if you agree)

> 
> The rest of the code movement looks alright to me.
> 
> Cheers,
> 
> -- 
> Julien Grall