* [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.