Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

[Andrew Cooper]

On 16/10/2026 6:06 pm, Teddy Astie wrote:
> This is no longer required since [1].
>
> At this moment of boot, per_cpu areas are not initialized, which prevents
> using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would
> misbehave in this case.

This isn't really correct.

The per-cpu area for the BSP is within the main image, which is why the
logic functions today.  What's missing is the call to
pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where
the BSP data is.

I think you want to say:

"A future change wants to use per_cpu_ptr_to_phys() in
xen_setup_runstate_info(), but this call is too early.  It turns out to
be no longer required since ..."

>
> [1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
> ---

Missing SoB.

~Andrew

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

[Andrew Cooper]

On 16/10/2026 6:33 pm, Andrew Cooper wrote:
> On 16/10/2026 6:06 pm, Teddy Astie wrote:
>> This is no longer required since [1].
>>
>> At this moment of boot, per_cpu areas are not initialized, which prevents
>> using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would
>> misbehave in this case.
> This isn't really correct.
>
> The per-cpu area for the BSP is within the main image, which is why the
> logic functions today.  What's missing is the call to
> pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where
> the BSP data is.
>
> I think you want to say:
>
> "A future change wants to use per_cpu_ptr_to_phys() in
> xen_setup_runstate_info(), but this call is too early.  It turns out to
> be no longer required since ..."
>
>> [1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
>> ---
> Missing SoB.
>
> ~Andrew

But this does pose another question...   Linux only ever registers time
referencing vCPU 0.  Why are the targets of the registration in the
per-cpu area?  That's presumably wasted space on all APs.

~Andrew

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 1437 bytes --]

Le 16/10/2026 à 20:04, Andrew Cooper a écrit :
> On 16/10/2026 6:33 pm, Andrew Cooper wrote:
>> On 16/10/2026 6:06 pm, Teddy Astie wrote:
>>> This is no longer required since [1].
>>>
>>> At this moment of boot, per_cpu areas are not initialized, which prevents
>>> using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would
>>> misbehave in this case.
>> This isn't really correct.
>>
>> The per-cpu area for the BSP is within the main image, which is why the
>> logic functions today.  What's missing is the call to
>> pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where
>> the BSP data is.
>>
>> I think you want to say:
>>
>> "A future change wants to use per_cpu_ptr_to_phys() in
>> xen_setup_runstate_info(), but this call is too early.  It turns out to
>> be no longer required since ..."
>>
>>> [1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
>>> ---
>> Missing SoB.
>>
>> ~Andrew
> 
> But this does pose another question...   Linux only ever registers time
> referencing vCPU 0.  Why are the targets of the registration in the
> per-cpu area?  That's presumably wasted space on all APs.
> 

Yes but only for this early PV case, there are other places where 
xen_setup_runstate_info() is called for other APs.

But looking at it, there may be redundant calls of 
xen_setup_runstate_info().

> ~Andrew

Teddy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

[Teddy Astie]

[-- Attachment #1.1: Type: text/plain, Size: 1098 bytes --]

Le 16/10/2026 à 19:34, Andrew Cooper a écrit :
> On 16/10/2026 6:06 pm, Teddy Astie wrote:
>> This is no longer required since [1].
>>
>> At this moment of boot, per_cpu areas are not initialized, which prevents
>> using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would
>> misbehave in this case.
> 
> This isn't really correct.
> 
> The per-cpu area for the BSP is within the main image, which is why the
> logic functions today.  What's missing is the call to
> pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where
> the BSP data is.
> 
> I think you want to say:
> 
> "A future change wants to use per_cpu_ptr_to_phys() in
> xen_setup_runstate_info(), but this call is too early.  It turns out to
> be no longer required since ..."
> 

Right, I was thinking that the use of the per_cpu was incorrect at this 
point. But that's not exactly the issue here.

Will adjust for the v3.

>>
>> [1] 8a22b9996b00 ("xen: drop xen_sched_clock in favour of using plain wallclock time")
>> ---
> 
> Missing SoB.
> 
> ~Andrew

Teddy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH v2 1/8] x86/xen/pv: Don't setup runstate area too early

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 1111 bytes --]

On 19.10.26 11:20, Teddy Astie wrote:
> Le 16/10/2026 à 19:34, Andrew Cooper a écrit :
>> On 16/10/2026 6:06 pm, Teddy Astie wrote:
>>> This is no longer required since [1].
>>>
>>> At this moment of boot, per_cpu areas are not initialized, which prevents
>>> using VCPUOP_register_runstate_phys_area as per_cpu_ptr_to_phys() would
>>> misbehave in this case.
>>
>> This isn't really correct.
>>
>> The per-cpu area for the BSP is within the main image, which is why the
>> logic functions today.  What's missing is the call to
>> pcpu_setup_first_chunk() which tells per_cpu_ptr_to_phys() about where
>> the BSP data is.
>>
>> I think you want to say:
>>
>> "A future change wants to use per_cpu_ptr_to_phys() in
>> xen_setup_runstate_info(), but this call is too early.  It turns out to
>> be no longer required since ..."
>>
> 
> Right, I was thinking that the use of the per_cpu was incorrect at this point. 
> But that's not exactly the issue here.
> 
> Will adjust for the v3.
With the suggested wording above:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 2/8] xen: Refactor xen/features{.c,.h}

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 468 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> Improve various aspects of xen/features{.c,.h}
>   * Use a bitmap instead of a array of u8 (effectively bool).
>   * Mark xen_setup_features as __init, as it's only called in __init
>     paths.
>   * Return bool instead of int in xen_feature()
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 3/8] xen: Add gpa conversion helpers

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 878 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> Introduce various helpers to convert between GPA and virt/paddr.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Reviewed-by: Juergen Gross <jgross@suse.com>

If you are up to it, there is some potential for cleaning up the Xen related
page.h include files.

You are adding the same definitions to the x86 and arm specific page.h files,
which are located at different positions regarding the arch trees.

I think this should be cleaned up by moving the common parts into
include/xen/page.h. The arch specific include files should both be under
include/xen/<arch>/page.h, I think (using arch/<arch>/include/asm/xen/page.h
won't work, as the arm file is used by arm and arm64).

In case you don't want to do that, I'm fine to do that later, e.g. when the
riscv Xen port is arriving in the kernel.


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 4/8] x86/xen/time: Correct xen_save_time_memory_area() failure message

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 324 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> We actually didn't fail to "save secondary vcpu time info" but
> just to unregister the mapping of it.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 5/8] x86/xen/time: Clear xen_clock mapping later

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 667 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> The current logic has a corner case when both xen_save_time_memory_area()
> and xen_restore_time_memory_area() fails. In this case, the mapping is never
> cleared and kept with previous pvclock values (invalidating the comment
> regarding VDSO_CLOCKMODE_PVCLOCK as the data may not have been zeroed).
> 
> Also convert the clear_page with a memset, there's no need to clear the
> entire page when we only care about 64 bytes of it.
> 
> Fixes: 2229f70b5bbb ("x86/xen/time: setup vcpu 0 time info page")
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 6/8] x86/xen/time: Mark xen_setup_vsyscall_time_info() __init

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 229 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> This function is only called by xen_time_init() being itself __init.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 7/8] x86/xen/time: Use VCPUOP_register_{runstate,vcpu_time}_phys_area when available

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 586 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> Updating these area can be expensive when done through virtual addresses
> (especially for runstate one), in particular for HVM/PVH guests.
> Xen 4.18 introduced physical address alternatives to address those
> short-commings, but they aren't used currently.
> 
> Fix it by using these alternatives when available (when reported in
> XENFEAT_{runstate,vcpu_time}_phys_area.
> 
> Assisted-by: LLM
> [guidance and testing]
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [RFC PATCH v2 8/8] xen/sysfs: Honor cached xen_features instead

[Juergen Gross]

[-- Attachment #1.1: Type: text/plain, Size: 992 bytes --]

On 16.10.26 19:06, Teddy Astie wrote:
> Currently, /sys/hypervisor/properties/features queries the xen
> features from the hypervisor even if it doesn't match what Linux
> uses itself (i.e what were the features at the time of the boot).
> 
> For consistency when migrations are involved, report cached
> xen_features instead, that also avoids making hypercalls to query
> this information.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> CC: Juergen Gross <jgross@suse.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> 
> I'm not sure what /sys/hypervisor/features is expected to show between
> what Linux considers (proposal) and what the hypervisor tells (current
> behavior).

I think it should be the former, so:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]