[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/pv: Make cr4_pv32_mask be PV32-only
On Fri, Aug 30, 2024 at 09:55:12AM +0200, Jan Beulich wrote: > On 29.08.2024 20:38, Andrew Cooper wrote: > > The user of cr4_pv32_mask (the cr4_pv32_restore() function) only exists in a > > CONFIG_PV32 build, but right now the variable is unconditionally set up. > > > > To start with, move the setup into set_in_cr4() and remove it from it's > > somewhat ad-hoc position in __start_xen(). This means the variable will be > > set up in two steps for a CONFIG_PV32=y build, but it's cleaner and more > > robust logic overall. > > > > With that, there's no good reason for the variable to stay in setup.c. Move > > it to x86/pv/traps.c (for want of any better place to live), and move the > > declaration to beside set_in_cr4() and mmu_cr4_features which is a better > > position than setup.h. > > > > Guard the reference with CONFIG_PV32, and fix up a recent typo in an > > adjacent > > comment while at it. > > > > No functional change. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> > with a suggestion: > > > --- a/xen/arch/x86/pv/traps.c > > +++ b/xen/arch/x86/pv/traps.c > > @@ -18,6 +18,10 @@ > > #include <asm/traps.h> > > #include <irq_vectors.h> > > > > +#ifdef CONFIG_PV32 > > +unsigned long __ro_after_init cr4_pv32_mask; > > +#endif > > To save on the number of such #ifdef-s, how about moving this into an existing > one. pv/mm.c has one, albeit near the bottom of the file (which I'd be fine > with, but I could imagine you or others not liking such placement), and > pv/domain.c has one near the top which seems pretty well suited. I'm fine either way: Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |