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;
And gate none.c on PV && !SHADOW_PAGING, which seems to be the only use.
It's a lot easier to see the safety on the HVM-only case, particularly with...
> is compiled out, and the toolstack has not specified the HAP flag at
> domain creation you will end up with a domain that doesn't have the
> paging operations initialized as paging_domain_init() would return 0
> with neither HAP nor shadow having been setup. That's likely to
> trigger NULL pointer dereferences inside of Xen.
>
> Also, seeing the code in arch_sanitise_domain_config() we possibly
> want to return an error at that point if toolstack attempts to create
> an HVM guest without HAP enabled, and shadow is build time disabled.
> I've sent a patch to that end.
... this patch you meantion. Thanks.
I'm guessing it's still a hot potato in for non-shadow PV, which strongly hints
at our being better off leaving it in that case. On HVM-only configurations it
seems rather silly.
Cheers,
Alejandro
