On Mon, Feb 09, 2026 at 05:04:55PM +0100, Jan Beulich wrote: > On 09.02.2026 16:55, Roger Pau Monné wrote: > > On Mon, Feb 09, 2026 at 04:35:04PM +0100, Alejandro Vallejo wrote: > >> On Mon Feb 9, 2026 at 3:42 PM CET, Roger Pau Monné wrote: > >>> On Mon, Feb 09, 2026 at 11:41:02AM +0100, Alejandro Vallejo wrote: > >>>> It only has 2 callers, both of which can be conditionally removed. > >>>> > >>>> Signed-off-by: Alejandro Vallejo <[email protected]> > >>>> --- > >>>> I'd be ok conditionalising the else branch on... > >>>> > >>>> IS_ENABLED(CONFIG_SHADOW_PAGING )|| IS_ENABLED(CONFIG_LOG_DIRTY) > >>>> > >>>> logdirty patch: > >>>> https://lore.kernel.org/xen-devel/[email protected] > >>>> > >>>> ... to avoid the danger of stale pointers, with required changes > >>>> elsewhere so > >>>> none.c is only compiled out in that case. > >>>> > >>>> I'm not sure how much it matters seeing how they are all unreachable. > >>>> --- > >>>> xen/arch/x86/mm/Makefile | 2 +- > >>>> xen/arch/x86/mm/paging.c | 4 +- > >>>> xen/arch/x86/mm/shadow/Makefile | 4 -- > >>>> xen/arch/x86/mm/shadow/none.c | 77 --------------------------------- > >>>> 4 files changed, 3 insertions(+), 84 deletions(-) > >>>> delete mode 100644 xen/arch/x86/mm/shadow/none.c > >>>> > >>>> diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile > >>>> index 960f6e8409..066c4caff3 100644 > >>>> --- a/xen/arch/x86/mm/Makefile > >>>> +++ b/xen/arch/x86/mm/Makefile > >>>> @@ -1,4 +1,4 @@ > >>>> -obj-y += shadow/ > >>>> +obj-$(CONFIG_SHADOW_PAGING) += shadow/ > >>>> obj-$(CONFIG_HVM) += hap/ > >>>> > >>>> obj-$(CONFIG_ALTP2M) += altp2m.o > >>>> diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c > >>>> index 2396f81ad5..5f70254cec 100644 > >>>> --- a/xen/arch/x86/mm/paging.c > >>>> +++ b/xen/arch/x86/mm/paging.c > >>>> @@ -634,7 +634,7 @@ int paging_domain_init(struct domain *d) > >>>> */ > >>>> if ( hap_enabled(d) ) > >>>> hap_domain_init(d); > >>>> - else > >>>> + else if ( IS_ENABLED(CONFIG_SHADOW_PAGING) ) > >>>> rc = shadow_domain_init(d); > >>> > >>> If you want to go this route you will need to set rc = -EOPNOTSUPP; > >>> prior to the `if ... else if` on the HVM case. > >> > >> Maybe this instead > >> > >> else > >> rc = IS_ENABLED(PV) ? shadow_domain_init(d) : -EOPNOTSUPP; > > > > But even for the PV case we cannot call shadow_domain_init() if shadow > > is compiled out? I think you want: > > > > if ( hap_enabled(d) ) > > hap_domain_init(d); > > else if ( IS_ENABLED(CONFIG_SHADOW_PAGING) ) > > rc = shadow_domain_init(d); > > else > > rc = is_hvm_domain(d) ? -EOPNOTSUPP : 0; > > Wouldn't this still leave NULL pointers in places where they can be rather > dangerous with PV guests?
Possibly, I didn't look much further on this patch, as returning rc == 0 and not having initialized neither HAP nor shadow is likely to already lead to NULL pointer dereferences. Regards, Roger.
