Re: [PATCH v7 02/19] xen/riscv: disable unnecessary configs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.04.2024 16:39, Oleksii wrote:
> On Wed, 2024-04-03 at 13:53 +0200, Jan Beulich wrote:
>> On 03.04.2024 12:54, Oleksii wrote:
>>> On Wed, 2024-04-03 at 12:28 +0200, Jan Beulich wrote:
>>>> On 03.04.2024 12:19, Oleksii Kurochko wrote:
>>>>> This patch disables unnecessary configs for two cases:
>>>>> 1. By utilizing EXTRA_FIXED_RANDCONFIG for randconfig builds
>>>>> (GitLab CI jobs).
>>>>> 2. By using tiny64_defconfig for non-randconfig builds.
>>>>>
>>>>> Only configs which lead to compilation issues were disabled.
>>>>>
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>>>>> ---
>>>>> Changes in V7:
>>>>>  - Disable only configs which cause compilation issues.
>>>>
>>>> Since the description doesn't go into details: While I can see
>>>> that
>>>> PERF_COUNTERS and LIVEPATCH may require (a little / some more)
>>>> extra
>>>> work, are HYPFS, ARGO, and XSM really causing issues?
>>> For Argo, I recieved the following compilation errors:
>>>    common/argo.c:1416:5: error: unknown type name 'p2m_type_t'; did
>>> you
>>>    mean 'hvmmem_type_t'?
>>>     1416 |     p2m_type_t p2mt;
>>>          |     ^~~~~~~~~~
>>>          |     hvmmem_type_t
>>>    common/argo.c:1419:11: error: implicit declaration of function
>>>    'check_get_page_from_gfn' [-Werror=implicit-function-
>>> declaration]
>>>     1419 |     ret = check_get_page_from_gfn(d, gfn, false, &p2mt,
>>> &page);
>>>          |           ^~~~~~~~~~~~~~~~~~~~~~~
>>>    common/argo.c:1427:10: error: 'p2m_ram_rw' undeclared (first use
>>> in
>>>    this function)
>>>     1427 |     case p2m_ram_rw:
>>>    
>>> It seems it should be included xen/p2m-common.h and asm/p2m.h in
>>> common/argo.c.
>>>
>>> For CONFIG_HYPFS_CONFIG ( there is no issue with CONFIG_HYPFS,
>>> overlooked that ):
>>>    common/config_data.S:1:10: fatal error: asm/asm_defns.h: No such
>>> file
>>>    or directory
>>>        1 | #include <asm/asm_defns.h>
>>>    
>>>
>>> For XSM, I recieved the following error:
>>>
>>>    xsm/xsm_core.c:79:19: error: 'xsm_core_init' defined but not
>>> used [-
>>>    Werror=unused-function]
>>>       79 | static int __init xsm_core_init(const void
>>> *policy_buffer,
>>>    size_t policy_size)
>>>
>>> I'll add an information with compilation errors to the commit
>>> message.
>>
>> No need to quote full compiler diagnostics, but a hint at the
>> problems
>> at least. That said, perhaps we want to rather sort the issues than
>> disable building stuff that sooner or later you will want to build
>> anyway. For hypfs we look to have an approach already. For Argo what
>> you suggest makes sense to me; it might be nice to understand where
>> the P2M headers needed are coming from on x86 and Arm. Ideally common
>> code .c files wouldn't include asm/*.h.
> It seems to me that p2m.h comes for Arm from argo.c -> xen/domain.h ->
> asm/domain.h and for x86 from argo.c -> xen/guest_access.h -> #include
> <asm/hvm/support.h> -> asm/p2m.h.
> 
> So I can include asm/p2m.h to asm/domain.h as p2m will be used anyway
> in asm/domain.h header and drop disablement of ARGO config from
> *_defconfig and build.yaml for CI. Does it make sense?

Looks okay to ma, at a glance.

Jan