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? Jan
