Re: [PATCH v4 20/24] xen/x86: wrap x86-specific domctl-op with CONFIG_MGMT_HYPERCALLS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.12.2025 04:28, Penny, Zheng wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Thursday, December 4, 2025 5:41 PM
>>
>> On 04.12.2025 05:23, Penny, Zheng wrote:
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>> Sent: Wednesday, November 26, 2025 1:00 AM
>>>>
>>>> On 21.11.2025 11:57, Penny Zheng wrote:
>>>>> --- a/xen/arch/x86/Kconfig
>>>>> +++ b/xen/arch/x86/Kconfig
>>>>> @@ -29,7 +29,7 @@ config X86
>>>>>     select HAS_PCI_MSI
>>>>>     select HAS_PIRQ
>>>>>     select HAS_SCHED_GRANULARITY
>>>>> -   imply HAS_SOFT_RESET
>>>>> +   select HAS_SOFT_RESET if MGMT_HYPERCALLS
>>>>
>>>> Why would you undo an "imply", when really we should use it more? One
>>>> of its purposes is so that you can add "depends on MGMT_HYPERCALLS"
>>>> there, and it then won't be auto-enabled if that dependency isn't 
>>>> fulfilled.
>>>>
>>>>> --- a/xen/arch/x86/include/asm/paging.h
>>>>> +++ b/xen/arch/x86/include/asm/paging.h
>>>>> @@ -55,7 +55,7 @@
>>>>>  #define PG_translate   0
>>>>>  #define PG_external    0
>>>>>  #endif
>>>>> -#if defined(CONFIG_PAGING) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
>>>>> +#if defined(CONFIG_PAGING)
>> && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
>>>> &&
>>>>> +defined(CONFIG_MGMT_HYPERCALLS)
>>>>
>>>> This line is too long now.
>>>
>>> Hmm, how about I introduce a new Kconfig, something like:
>>> ```
>>> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index
>>> c808c989fc..ec8f51b0b1 100644
>>> --- a/xen/arch/x86/Kconfig
>>> +++ b/xen/arch/x86/Kconfig
>>> @@ -166,6 +166,9 @@ config SHADOW_PAGING  config PAGING
>>>         def_bool HVM || SHADOW_PAGING
>>>
>>> +config PAGING_LOG_DIRTY
>>> +       def_bool PAGING
>>> +
>>>  config BIGMEM
>>>         bool "big memory support"
>>>         default n
>>> ```
>>> Then, later future dependency could be added to PAGING_LOG_DIRTY
>>
>> I fear I don't see how having an option merely aliasing another option would 
>> help the
>> line length issue. I may simply be missing some context ...
> 
> Later when MGMT_HYPERCCALLS kicks in, we could have to avoid overlong #ifdef:
> ```
> #if defined(CONFIG_PAGING_LOG_DIRTY) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)

Yet this is a bogus construct to have. It really means PAGING_LOG_DIRTY would
better depend on !PV_SHIM_EXCLUSIVE, except that this is precisely the
pattern we want to get rid of. At which point imo the usefulness of
PAGING_LOG_DIRTY would be somewhat limited.

Jan

> #define PG_log_dirty   (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
> ...
> ```
> The addition of MGMT_HYPERCCALLS dependency could be added to 
> PAGING_LOG_DIRTY kconfig
> ```
> config PAGING_LOG_DIRTY
>         def_bool PAGING && MGMT_HYPERCALLS
> ```
> 
>>
>> Jan