Re: [PATCH] xen/x86: restrict XENFEAT_pae_pgdir_above_4gb setting to PV guests

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

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.