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

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Thursday, December 4, 2025 5:41 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; grygorii_strashko@xxxxxxxx; Andrew
> Cooper <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Orzel, Michal <Michal.Orzel@xxxxxxx>; Julien
> Grall <julien@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>; Stefano
> Stabellini <sstabellini@xxxxxxxxxx>; Daniel P. Smith
> <dpsmith@xxxxxxxxxxxxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 20/24] xen/x86: wrap x86-specific domctl-op with
> CONFIG_MGMT_HYPERCALLS
>
> On 04.12.2025 05:23, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Wednesday, November 26, 2025 1:00 AM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; grygorii_strashko@xxxxxxxx;
> >> Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD
> >> <anthony.perard@xxxxxxxxxx>; Orzel, Michal <Michal.Orzel@xxxxxxx>;
> >> Julien Grall <julien@xxxxxxx>; Roger Pau Monné
> >> <roger.pau@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> >> Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>;
> >> xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v4 20/24] xen/x86: wrap x86-specific domctl-op
> >> with CONFIG_MGMT_HYPERCALLS
> >>
> >> 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)
#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