xen/arch/x86/include/asm/mm.h | 1 - xen/arch/x86/pv/domain.c | 6 +++++- xen/arch/x86/x86_64/mm.c | 13 ------------- 3 files changed, 5 insertions(+), 15 deletions(-)
Prior to commit 02e78311cdc6 ("x86/domctl: Make XEN_DOMCTL_set_address_size
singleshot") (Xen 4.9, 2016), it was possible for domains to switch to being
compat, and back again. Since then however, becoming compat is a singleton
action that can't be undone.
From the context it's clear to see the is_pv_32bit_domain() check is
redundant, and from the singleton nature being the only place setting
physaddr_bitsize, there's no need to check it for being 0.
No functional change.
Co-developed-by: Grygorii Strashko <grygorii_strashko@epam.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Grygorii Strashko <grygorii_strashko@epam.com>
Split out of series to simplify things.
bloat-o-meter reports:
add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
Function old new delta
switch_compat 447 472 +25
domain_set_alloc_bitsize 96 - -96
which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
---
xen/arch/x86/include/asm/mm.h | 1 -
xen/arch/x86/pv/domain.c | 6 +++++-
xen/arch/x86/x86_64/mm.c | 13 -------------
3 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 17ca6666a34e..9438f5ea0119 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -619,7 +619,6 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
-void domain_set_alloc_bitsize(struct domain *d);
unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits);
#define domain_clamp_alloc_bitsize(d, bits) domain_clamp_alloc_bitsize(d, bits)
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 9c4785c187dd..11db6a6d8396 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -254,7 +254,11 @@ int switch_compat(struct domain *d)
goto undo_and_fail;
}
- domain_set_alloc_bitsize(d);
+ if ( MACH2PHYS_COMPAT_NR_ENTRIES(d) < max_page )
+ d->arch.physaddr_bitsize =
+ /* 2^n entries can be contained in guest's p2m mapping space */
+ fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1 + PAGE_SHIFT;
+
recalculate_cpuid_policy(d);
d->arch.x87_fip_width = 4;
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index d4e6a9c0a2e0..42fd4fe4e9b5 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1119,19 +1119,6 @@ int handle_memadd_fault(unsigned long addr, struct cpu_user_regs *regs)
return ret;
}
-void domain_set_alloc_bitsize(struct domain *d)
-{
- if ( !is_pv_32bit_domain(d) ||
- (MACH2PHYS_COMPAT_NR_ENTRIES(d) >= max_page) ||
- d->arch.physaddr_bitsize > 0 )
- return;
- d->arch.physaddr_bitsize =
- /* 2^n entries can be contained in guest's p2m mapping space */
- fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1
- /* 2^n pages -> 2^(n+PAGE_SHIFT) bits */
- + PAGE_SHIFT;
-}
-
unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits)
{
if ( (d == NULL) || (d->arch.physaddr_bitsize == 0) )
--
2.39.5
Hi Andrew,
On 09.12.25 20:07, Andrew Cooper wrote:
> Prior to commit 02e78311cdc6 ("x86/domctl: Make XEN_DOMCTL_set_address_size
> singleshot") (Xen 4.9, 2016), it was possible for domains to switch to being
> compat, and back again. Since then however, becoming compat is a singleton
> action that can't be undone.
>
> From the context it's clear to see the is_pv_32bit_domain() check is
> redundant, and from the singleton nature being the only place setting
> physaddr_bitsize, there's no need to check it for being 0.
>
> No functional change.
>
> Co-developed-by: Grygorii Strashko <grygorii_strashko@epam.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Grygorii Strashko <grygorii_strashko@epam.com>
>
> Split out of series to simplify things.
>
> bloat-o-meter reports:
>
> add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
> Function old new delta
> switch_compat 447 472 +25
> domain_set_alloc_bitsize 96 - -96
>
> which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
Thank you for doing this.
Not sure if it's needed, any way.
Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
> xen/arch/x86/include/asm/mm.h | 1 -
> xen/arch/x86/pv/domain.c | 6 +++++-
> xen/arch/x86/x86_64/mm.c | 13 -------------
> 3 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
> index 17ca6666a34e..9438f5ea0119 100644
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -619,7 +619,6 @@ void __iomem *ioremap_wc(paddr_t pa, size_t len);
>
> extern int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm);
>
> -void domain_set_alloc_bitsize(struct domain *d);
> unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits);
> #define domain_clamp_alloc_bitsize(d, bits) domain_clamp_alloc_bitsize(d, bits)
>
> diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
> index 9c4785c187dd..11db6a6d8396 100644
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -254,7 +254,11 @@ int switch_compat(struct domain *d)
> goto undo_and_fail;
> }
>
> - domain_set_alloc_bitsize(d);
> + if ( MACH2PHYS_COMPAT_NR_ENTRIES(d) < max_page )
> + d->arch.physaddr_bitsize =
> + /* 2^n entries can be contained in guest's p2m mapping space */
> + fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1 + PAGE_SHIFT;
> +
> recalculate_cpuid_policy(d);
>
> d->arch.x87_fip_width = 4;
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index d4e6a9c0a2e0..42fd4fe4e9b5 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1119,19 +1119,6 @@ int handle_memadd_fault(unsigned long addr, struct cpu_user_regs *regs)
> return ret;
> }
>
> -void domain_set_alloc_bitsize(struct domain *d)
> -{
> - if ( !is_pv_32bit_domain(d) ||
> - (MACH2PHYS_COMPAT_NR_ENTRIES(d) >= max_page) ||
> - d->arch.physaddr_bitsize > 0 )
> - return;
> - d->arch.physaddr_bitsize =
> - /* 2^n entries can be contained in guest's p2m mapping space */
> - fls(MACH2PHYS_COMPAT_NR_ENTRIES(d)) - 1
> - /* 2^n pages -> 2^(n+PAGE_SHIFT) bits */
> - + PAGE_SHIFT;
> -}
> -
> unsigned int domain_clamp_alloc_bitsize(struct domain *d, unsigned int bits)
> {
> if ( (d == NULL) || (d->arch.physaddr_bitsize == 0) )
--
Best regards,
-grygorii
On 09/12/2025 7:21 pm, Grygorii Strashko wrote:
> Hi Andrew,
>
> On 09.12.25 20:07, Andrew Cooper wrote:
>> Prior to commit 02e78311cdc6 ("x86/domctl: Make
>> XEN_DOMCTL_set_address_size
>> singleshot") (Xen 4.9, 2016), it was possible for domains to switch
>> to being
>> compat, and back again. Since then however, becoming compat is a
>> singleton
>> action that can't be undone.
>>
>> From the context it's clear to see the is_pv_32bit_domain() check is
>> redundant, and from the singleton nature being the only place setting
>> physaddr_bitsize, there's no need to check it for being 0.
>>
>> No functional change.
>>
>> Co-developed-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> Split out of series to simplify things.
>>
>> bloat-o-meter reports:
>>
>> add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
>> Function old new delta
>> switch_compat 447 472 +25
>> domain_set_alloc_bitsize 96 - -96
>>
>> which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
>
> Thank you for doing this.
> Not sure if it's needed, any way.
> Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
It does help. Technically it lets me commit the patch right now, but
I'll leave it until at least tomorrow in case anyone else has comments.
~Andrew
On 09.12.2025 20:29, Andrew Cooper wrote:
> On 09/12/2025 7:21 pm, Grygorii Strashko wrote:
>> Hi Andrew,
>>
>> On 09.12.25 20:07, Andrew Cooper wrote:
>>> Prior to commit 02e78311cdc6 ("x86/domctl: Make
>>> XEN_DOMCTL_set_address_size
>>> singleshot") (Xen 4.9, 2016), it was possible for domains to switch
>>> to being
>>> compat, and back again. Since then however, becoming compat is a
>>> singleton
>>> action that can't be undone.
>>>
>>> From the context it's clear to see the is_pv_32bit_domain() check is
>>> redundant, and from the singleton nature being the only place setting
>>> physaddr_bitsize, there's no need to check it for being 0.
>>>
>>> No functional change.
>>>
>>> Co-developed-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Grygorii Strashko <grygorii_strashko@epam.com>
>>>
>>> Split out of series to simplify things.
>>>
>>> bloat-o-meter reports:
>>>
>>> add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
>>> Function old new delta
>>> switch_compat 447 472 +25
>>> domain_set_alloc_bitsize 96 - -96
>>>
>>> which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
>>
>> Thank you for doing this.
>> Not sure if it's needed, any way.
>> Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
>
> It does help. Technically it lets me commit the patch right now, but
> I'll leave it until at least tomorrow in case anyone else has comments.
Just to confirm:
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
On 09/12/2025 7:29 pm, Andrew Cooper wrote:
> On 09/12/2025 7:21 pm, Grygorii Strashko wrote:
>> Hi Andrew,
>>
>> On 09.12.25 20:07, Andrew Cooper wrote:
>>> Prior to commit 02e78311cdc6 ("x86/domctl: Make
>>> XEN_DOMCTL_set_address_size
>>> singleshot") (Xen 4.9, 2016), it was possible for domains to switch
>>> to being
>>> compat, and back again. Since then however, becoming compat is a
>>> singleton
>>> action that can't be undone.
>>>
>>> From the context it's clear to see the is_pv_32bit_domain() check is
>>> redundant, and from the singleton nature being the only place setting
>>> physaddr_bitsize, there's no need to check it for being 0.
>>>
>>> No functional change.
>>>
>>> Co-developed-by: Grygorii Strashko <grygorii_strashko@epam.com>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Grygorii Strashko <grygorii_strashko@epam.com>
>>>
>>> Split out of series to simplify things.
>>>
>>> bloat-o-meter reports:
>>>
>>> add/remove: 0/1 grow/shrink: 1/0 up/down: 25/-96 (-71)
>>> Function old new delta
>>> switch_compat 447 472 +25
>>> domain_set_alloc_bitsize 96 - -96
>>>
>>> which will mostly be the LFENCEs embedded in is_pv_32bit_domain().
>> Thank you for doing this.
>> Not sure if it's needed, any way.
>> Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
> It does help. Technically it lets me commit the patch right now, but
> I'll leave it until at least tomorrow in case anyone else has comments.
>
> ~Andrew
>
FYI, here is the remainder of your patch rebased over this one.
I recommend splitting it into two, one sorting out
domain_clamp_alloc_bitsize() (however that will end up looking), and one
moving physaddr_bitsize into pv_domain.
It's almost always better to separate code movement from logical changes.
~Andrew
© 2016 - 2025 Red Hat, Inc.