* [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
@ 2026-09-30 19:11 Roger Pau Monne
2026-09-30 20:42 ` Teddy Astie
2026-10-01 7:34 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Roger Pau Monne @ 2026-09-30 19:11 UTC (permalink / raw)
To: xen-devel
Cc: Roger Pau Monne, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Stefano Stabellini
XENFEAT_pae_pgdir_above_4gb is only used by 32bit PV guests, hence limit
its setting in XENVER_get_features to the is_pv_domain() guarded section,
with the rest of the PV specific features.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
---
xen/common/kernel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index f4bbd8818fa8..95cf56827317 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -677,8 +677,6 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
(1U << XENFEAT_vcpu_time_phys_area) |
#endif
(1U << XENFEAT_runstate_phys_area);
- if ( VM_ASSIST(d, pae_extended_cr3) )
- fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
if ( paging_mode_translate(d) )
fi.submap |=
(1U << XENFEAT_writable_page_tables) |
@@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( is_pv_domain(d) )
fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
(1U << XENFEAT_highmem_assist) |
- (1U << XENFEAT_gnttab_map_avail_bits);
+ (1U << XENFEAT_gnttab_map_avail_bits) |
+ (VM_ASSIST(d, pae_extended_cr3) ?
+ (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
else
fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
(1U << XENFEAT_hvm_callback_vector) |
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
2026-09-30 19:11 [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests Roger Pau Monne
@ 2026-09-30 20:42 ` Teddy Astie
2026-10-01 7:34 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Teddy Astie @ 2026-09-30 20:42 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich,
Julien Grall, Stefano Stabellini
[-- Attachment #1.1: Type: text/plain, Size: 1896 bytes --]
Le 30/09/2026 à 21:14, Roger Pau Monne a écrit :
> XENFEAT_pae_pgdir_above_4gb is only used by 32bit PV guests, hence limit
> its setting in XENVER_get_features to the is_pv_domain() guarded section,
> with the rest of the PV specific features.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger@xenproject.org>
> ---
> xen/common/kernel.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index f4bbd8818fa8..95cf56827317 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -677,8 +677,6 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> (1U << XENFEAT_vcpu_time_phys_area) |
> #endif
> (1U << XENFEAT_runstate_phys_area);
> - if ( VM_ASSIST(d, pae_extended_cr3) )
> - fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
> if ( paging_mode_translate(d) )
> fi.submap |=
> (1U << XENFEAT_writable_page_tables) |
> @@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> if ( is_pv_domain(d) )
> fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
> (1U << XENFEAT_highmem_assist) |
> - (1U << XENFEAT_gnttab_map_avail_bits);
> + (1U << XENFEAT_gnttab_map_avail_bits) |
> + (VM_ASSIST(d, pae_extended_cr3) ?
> + (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
> else
> fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
> (1U << XENFEAT_hvm_callback_vector) |
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Teddy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
2026-09-30 19:11 [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests Roger Pau Monne
2026-09-30 20:42 ` Teddy Astie
@ 2026-10-01 7:34 ` Jan Beulich
2026-10-01 8:29 ` Roger Pau Monné
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2026-10-01 7:34 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 30.09.2026 21:11, Roger Pau Monne wrote:
> @@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> if ( is_pv_domain(d) )
> fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
> (1U << XENFEAT_highmem_assist) |
> - (1U << XENFEAT_gnttab_map_avail_bits);
> + (1U << XENFEAT_gnttab_map_avail_bits) |
> + (VM_ASSIST(d, pae_extended_cr3) ?
> + (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
Nit: Unlike binary operators, which we want to have placed at the end of
wrapped lines, we generally prefer the ternary operator to be wrapped
differently. E.g.
(VM_ASSIST(d, pae_extended_cr3)
? (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
or
(VM_ASSIST(d, pae_extended_cr3)
? (1U << XENFEAT_pae_pgdir_above_4gb)
: 0);
This isn't written down anywhere, so I'm not going to insist.
Furthermore, wouldn't it be plausible to also add IS_ENABLED(CONFIG_PV32)
(or, imo less desirably, opt_pv32)?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
2026-10-01 7:34 ` Jan Beulich
@ 2026-10-01 8:29 ` Roger Pau Monné
2026-10-01 9:09 ` Andrew Cooper
2026-10-01 11:00 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Roger Pau Monné @ 2026-10-01 8:29 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On Thu, Oct 01, 2026 at 09:34:34AM +0200, Jan Beulich wrote:
> On 30.09.2026 21:11, Roger Pau Monne wrote:
> > @@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> > if ( is_pv_domain(d) )
> > fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
> > (1U << XENFEAT_highmem_assist) |
> > - (1U << XENFEAT_gnttab_map_avail_bits);
> > + (1U << XENFEAT_gnttab_map_avail_bits) |
> > + (VM_ASSIST(d, pae_extended_cr3) ?
> > + (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>
> Nit: Unlike binary operators, which we want to have placed at the end of
> wrapped lines, we generally prefer the ternary operator to be wrapped
> differently. E.g.
>
> (VM_ASSIST(d, pae_extended_cr3)
> ? (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>
> or
>
> (VM_ASSIST(d, pae_extended_cr3)
> ? (1U << XENFEAT_pae_pgdir_above_4gb)
> : 0);
>
> This isn't written down anywhere, so I'm not going to insist.
I had it written as your first proposal, but then moved the ternary
operator at the end of the line as I've assumed it would be prefer to
match the position of the binary ones.
> Furthermore, wouldn't it be plausible to also add IS_ENABLED(CONFIG_PV32)
> (or, imo less desirably, opt_pv32)?
I could do:
if ( IS_ENABLED(CONFIG_PV32) && VM_ASSIST(d, pae_extended_cr3) )
fi.submap |= 1U << XENFEAT_pae_pgdir_above_4gb
As I think adding the IS_ENABLED(CONFIG_PV32) check to the previous
expression is too much.
Let me know your preference.
Thanks, Roger.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
2026-10-01 8:29 ` Roger Pau Monné
@ 2026-10-01 9:09 ` Andrew Cooper
2026-10-01 11:00 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2026-10-01 9:09 UTC (permalink / raw)
To: Roger Pau Monné, Jan Beulich
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 01/10/2026 9:29 am, Roger Pau Monné wrote:
> On Thu, Oct 01, 2026 at 09:34:34AM +0200, Jan Beulich wrote:
>> On 30.09.2026 21:11, Roger Pau Monne wrote:
>>> @@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>> if ( is_pv_domain(d) )
>>> fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
>>> (1U << XENFEAT_highmem_assist) |
>>> - (1U << XENFEAT_gnttab_map_avail_bits);
>>> + (1U << XENFEAT_gnttab_map_avail_bits) |
>>> + (VM_ASSIST(d, pae_extended_cr3) ?
>>> + (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>> Nit: Unlike binary operators, which we want to have placed at the end of
>> wrapped lines, we generally prefer the ternary operator to be wrapped
>> differently. E.g.
>>
>> (VM_ASSIST(d, pae_extended_cr3)
>> ? (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>>
>> or
>>
>> (VM_ASSIST(d, pae_extended_cr3)
>> ? (1U << XENFEAT_pae_pgdir_above_4gb)
>> : 0);
>>
>> This isn't written down anywhere, so I'm not going to insist.
> I had it written as your first proposal, but then moved the ternary
> operator at the end of the line as I've assumed it would be prefer to
> match the position of the binary ones.
>
>> Furthermore, wouldn't it be plausible to also add IS_ENABLED(CONFIG_PV32)
>> (or, imo less desirably, opt_pv32)?
> I could do:
>
> if ( IS_ENABLED(CONFIG_PV32) && VM_ASSIST(d, pae_extended_cr3) )
> fi.submap |= 1U << XENFEAT_pae_pgdir_above_4gb
>
> As I think adding the IS_ENABLED(CONFIG_PV32) check to the previous
> expression is too much.
>
> Let me know your preference.
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index f4bbd8818fa8..6811599f95fd 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -677,8 +677,6 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
(1U << XENFEAT_vcpu_time_phys_area) |
#endif
(1U << XENFEAT_runstate_phys_area);
- if ( VM_ASSIST(d, pae_extended_cr3) )
- fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
if ( paging_mode_translate(d) )
fi.submap |=
(1U << XENFEAT_writable_page_tables) |
@@ -690,9 +688,13 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
#endif
#ifdef CONFIG_X86
if ( is_pv_domain(d) )
+ {
fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
(1U << XENFEAT_highmem_assist) |
(1U << XENFEAT_gnttab_map_avail_bits);
+ if ( VM_ASSIST(d, pae_extended_cr3) )
+ fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
+ }
else
fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
(1U << XENFEAT_hvm_callback_vector) |
Please don't try to be clever trying to fold this logic. All that does
is require someone to revert it later when it inevitably gets too
complicated follow sensibly.
~Andrew
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests
2026-10-01 8:29 ` Roger Pau Monné
2026-10-01 9:09 ` Andrew Cooper
@ 2026-10-01 11:00 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2026-10-01 11:00 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 01.10.2026 10:29, Roger Pau Monné wrote:
> On Thu, Oct 01, 2026 at 09:34:34AM +0200, Jan Beulich wrote:
>> On 30.09.2026 21:11, Roger Pau Monne wrote:
>>> @@ -692,7 +690,9 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>> if ( is_pv_domain(d) )
>>> fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
>>> (1U << XENFEAT_highmem_assist) |
>>> - (1U << XENFEAT_gnttab_map_avail_bits);
>>> + (1U << XENFEAT_gnttab_map_avail_bits) |
>>> + (VM_ASSIST(d, pae_extended_cr3) ?
>>> + (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>>
>> Nit: Unlike binary operators, which we want to have placed at the end of
>> wrapped lines, we generally prefer the ternary operator to be wrapped
>> differently. E.g.
>>
>> (VM_ASSIST(d, pae_extended_cr3)
>> ? (1U << XENFEAT_pae_pgdir_above_4gb) : 0);
>>
>> or
>>
>> (VM_ASSIST(d, pae_extended_cr3)
>> ? (1U << XENFEAT_pae_pgdir_above_4gb)
>> : 0);
>>
>> This isn't written down anywhere, so I'm not going to insist.
>
> I had it written as your first proposal, but then moved the ternary
> operator at the end of the line as I've assumed it would be prefer to
> match the position of the binary ones.
>
>> Furthermore, wouldn't it be plausible to also add IS_ENABLED(CONFIG_PV32)
>> (or, imo less desirably, opt_pv32)?
>
> I could do:
>
> if ( IS_ENABLED(CONFIG_PV32) && VM_ASSIST(d, pae_extended_cr3) )
> fi.submap |= 1U << XENFEAT_pae_pgdir_above_4gb
>
> As I think adding the IS_ENABLED(CONFIG_PV32) check to the previous
> expression is too much.
>
> Let me know your preference.
I'm okay with the if() variant, but as you know I'm also not afraid of
long-ish ternary expressions.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-01 11:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30 19:11 [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests Roger Pau Monne
2026-09-30 20:42 ` Teddy Astie
2026-10-01 7:34 ` Jan Beulich
2026-10-01 8:29 ` Roger Pau Monné
2026-10-01 9:09 ` Andrew Cooper
2026-10-01 11:00 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.