Re: [PATCH v2 04/26] xen: consolidate CONFIG_VM_EVENT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.09.2025 08:39, Penny, Zheng wrote:
> [Public]
> 
> Hi,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Sunday, September 14, 2025 10:04 PM
>> To: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Penny, Zheng
>> <penny.zheng@xxxxxxx>
>> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
>> Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>; Petre Pircalabu
>> <ppircalabu@xxxxxxxxxxxxxxx>; Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>;
>> xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2 04/26] xen: consolidate CONFIG_VM_EVENT
>>
>> On 14.09.2025 01:31, Tamas K Lengyel wrote:
>>>>> @@ -99,10 +98,40 @@ long p2m_set_mem_access_multi(struct domain *d,
>>>>> int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
>> *access,
>>>>>                         unsigned int altp2m_idx);
>>>>>
>>>>> -#ifdef CONFIG_VM_EVENT
>>>>>  int mem_access_memop(unsigned long cmd,
>>>>>                       XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t)
>>>>> arg);  #else
>>>>> +static inline bool xenmem_access_to_p2m_access(const struct p2m_domain
>> *p2m,
>>>>> +                                               xenmem_access_t xaccess,
>>>>> +                                               p2m_access_t
>>>>> +*paccess) {
>>>>> +    return false;
>>>>> +}
>>>>
>>>> So this is needed when VM_EVENT=n and ALTP2M=y. Tamas, is this a
>>>> configuration which makes sense?
>>>
>>> Yes, altp2m should be functional without vm_event being enabled. There
>>> could very well be in-guest only use of altp2m via #VE. This function
>>> is used in p2m_init_next_altp2m which means it being stubbed out like
>>> this when vm_event is disabled breaks altp2m.
>>
>> Oh, indeed - the stub still needs to handle XENMEM_access_default. Of course
>> with MEM_ACCESS=n it's not quite clear to me what p2m->default_access ought
>> to be; imo in principle that field ought to also go away in that case 
>> (becoming hard-
>> coded p2m_access_rwx). While doing that will be a larger patch, perhaps 
>> using the
>> hard-coded value here should be done right away.
>>
>> Once the code correctly handles MEM_ACCESS=n as an implication from
>> VM_EVENT=n, it's also questionable whether MEM_ACCESS_ALWAYS_ON
>> should be retained.
>>
> 
> If we intend to remove MEM_ACCESS_ALWAYS_ON, I suggest to do the following 
> modification on VM_EVENT to still keep y on default on x86:
> ```
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 7bd8a04730..61d48a5120 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -170,13 +170,10 @@ config HAS_VMAP
>  config LIBFDT
>         bool
> 
> -config MEM_ACCESS_ALWAYS_ON
> -       bool
> -
>  config VM_EVENT
> -       def_bool MEM_ACCESS_ALWAYS_ON
> -       prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
> +       bool "Memory Access and VM events"
>         depends on HVM
> +       default X86
>         help
> 
>           Framework to configure memory access types for guests and receive
> ```

Yes (at least for the time being; eventually we may want to make this default N
even on x86).

Jan