* [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG
@ 2022-12-01 10:03 Ayan Kumar Halder
2022-12-01 10:26 ` Julien Grall
0 siblings, 1 reply; 5+ messages in thread
From: Ayan Kumar Halder @ 2022-12-01 10:03 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, stefanos, julien, andrew.cooper3, george.dunlap,
jbeulich, bobbyeshleman, alistair.francis, connojdavis, wl,
Ayan Kumar Halder
It is possible for a pointer to represent physical memory of the same size.
In other words, a 32 bit pointer can represent 32 bit addressable physical
memory.
Thus, issue a compilation failure only when the count of physical address bits
is greater than BITS_PER_LONG (ie count of bits in void*).
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Currently this change will not have any impact on the existing architectures.
The following table illustrates PADDR_BITS vs BITS_PER_LONG of different archs
------------------------------------------------
| Arch | PADDR_BITS | BITS_PER_LONG |
------------------------------------------------
| Arm_64 | 48 | 64 |
| Arm_32 | 40 | 32 |
| RISCV_64 | Don't know | 64 |
| x86 | 52 | 64 |
-------------------------------------------------
However, this will change when we introduce a platform (For eg Cortex-R52) which
supports 32 bit physical address and BITS_PER_LONG.
Thus, I have introduced this change as I don't see it causing a regression on
any of the supported platforms.
xen/common/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62afb07bc6..cd390a0956 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
{
ASSERT(!first_node_initialised);
ASSERT(!xenheap_bits);
- BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
+ BUILD_BUG_ON(PADDR_BITS > BITS_PER_LONG);
xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG
2022-12-01 10:03 [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG Ayan Kumar Halder
@ 2022-12-01 10:26 ` Julien Grall
2022-12-01 11:44 ` Jan Beulich
2022-12-01 12:12 ` Ayan Kumar Halder
0 siblings, 2 replies; 5+ messages in thread
From: Julien Grall @ 2022-12-01 10:26 UTC (permalink / raw)
To: Ayan Kumar Halder, xen-devel
Cc: sstabellini, stefanos, andrew.cooper3, george.dunlap, jbeulich,
bobbyeshleman, alistair.francis, connojdavis, wl
Hi Ayan,
On 01/12/2022 10:03, Ayan Kumar Halder wrote:
> It is possible for a pointer to represent physical memory of the same size.
> In other words, a 32 bit pointer can represent 32 bit addressable physical
> memory.
> Thus, issue a compilation failure only when the count of physical address bits
> is greater than BITS_PER_LONG (ie count of bits in void*).
I am having difficult to understand how this description is related to
the BUILD_BUG_ON(). AFAIU, it is used to check that xenheap_bits can be
used in shift.
If the unsigned long is 32-bit, then a shift of 32 could be undefined.
Looking at the current use, the shift are used with "xenheap_bits -
PAGE_SHIFT". So as long as PAGE_SHIFT is not 0, you would be fine.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
>
> Currently this change will not have any impact on the existing architectures.
> The following table illustrates PADDR_BITS vs BITS_PER_LONG of different archs
>
> ------------------------------------------------
> | Arch | PADDR_BITS | BITS_PER_LONG |
> ------------------------------------------------
> | Arm_64 | 48 | 64 |
> | Arm_32 | 40 | 32 |
> | RISCV_64 | Don't know | 64 |
> | x86 | 52 | 64 |
> -------------------------------------------------
The Arm_32 line is a bit confusing because one would wonder why we
haven't seen this issue yet. So I think you want to clarify that the
code path is not used by Arm32.
>
> However, this will change when we introduce a platform (For eg Cortex-R52) which
> supports 32 bit physical address and BITS_PER_LONG.
> Thus, I have introduced this change as I don't see it causing a regression on
> any of the supported platforms.
>
> xen/common/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 62afb07bc6..cd390a0956 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
> {
> ASSERT(!first_node_initialised);
> ASSERT(!xenheap_bits);
> - BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
> + BUILD_BUG_ON(PADDR_BITS > BITS_PER_LONG);
Based on the above, I think this wants to be "(PADDR_BITS - PAGE_SHIFT)
>= BITS_PER_LONG)".
> xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
> printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
> }
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG
2022-12-01 10:26 ` Julien Grall
@ 2022-12-01 11:44 ` Jan Beulich
2022-12-01 12:12 ` Ayan Kumar Halder
1 sibling, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2022-12-01 11:44 UTC (permalink / raw)
To: Julien Grall, Ayan Kumar Halder
Cc: sstabellini, stefanos, andrew.cooper3, george.dunlap,
bobbyeshleman, alistair.francis, connojdavis, wl, xen-devel
On 01.12.2022 11:26, Julien Grall wrote:
> On 01/12/2022 10:03, Ayan Kumar Halder wrote:
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
>> {
>> ASSERT(!first_node_initialised);
>> ASSERT(!xenheap_bits);
>> - BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
>> + BUILD_BUG_ON(PADDR_BITS > BITS_PER_LONG);
>
> Based on the above, I think this wants to be "(PADDR_BITS - PAGE_SHIFT)
> >= BITS_PER_LONG)".
+1, fwiw.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG
2022-12-01 10:26 ` Julien Grall
2022-12-01 11:44 ` Jan Beulich
@ 2022-12-01 12:12 ` Ayan Kumar Halder
2022-12-01 17:37 ` Julien Grall
1 sibling, 1 reply; 5+ messages in thread
From: Ayan Kumar Halder @ 2022-12-01 12:12 UTC (permalink / raw)
To: Julien Grall, Ayan Kumar Halder, xen-devel
Cc: sstabellini, stefanos, andrew.cooper3, george.dunlap, jbeulich,
bobbyeshleman, alistair.francis, connojdavis, wl
On 01/12/2022 10:26, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
I have a question.
>
> On 01/12/2022 10:03, Ayan Kumar Halder wrote:
>> It is possible for a pointer to represent physical memory of the same
>> size.
>> In other words, a 32 bit pointer can represent 32 bit addressable
>> physical
>> memory.
>> Thus, issue a compilation failure only when the count of physical
>> address bits
>> is greater than BITS_PER_LONG (ie count of bits in void*).
>
> I am having difficult to understand how this description is related to
> the BUILD_BUG_ON(). AFAIU, it is used to check that xenheap_bits can
> be used in shift.
>
> If the unsigned long is 32-bit, then a shift of 32 could be undefined.
> Looking at the current use, the shift are used with "xenheap_bits -
> PAGE_SHIFT". So as long as PAGE_SHIFT is not 0, you would be fine.
Ack
>
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>
>> Currently this change will not have any impact on the existing
>> architectures.
>> The following table illustrates PADDR_BITS vs BITS_PER_LONG of
>> different archs
>>
>> ------------------------------------------------
>> | Arch | PADDR_BITS | BITS_PER_LONG |
>> ------------------------------------------------
>> | Arm_64 | 48 | 64 |
>> | Arm_32 | 40 | 32 |
>> | RISCV_64 | Don't know | 64 |
>> | x86 | 52 | 64 |
>> -------------------------------------------------
>
> The Arm_32 line is a bit confusing because one would wonder why we
> haven't seen this issue yet. So I think you want to clarify that the
> code path is not used by Arm32.
Do you want this clarification and the above/below explanation to be a
part of the commit message ?
I will then send out a v2 with the fix.
- Ayan
>
>>
>> However, this will change when we introduce a platform (For eg
>> Cortex-R52) which
>> supports 32 bit physical address and BITS_PER_LONG.
>> Thus, I have introduced this change as I don't see it causing a
>> regression on
>> any of the supported platforms.
>>
>> xen/common/page_alloc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>> index 62afb07bc6..cd390a0956 100644
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
>> {
>> ASSERT(!first_node_initialised);
>> ASSERT(!xenheap_bits);
>> - BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
>> + BUILD_BUG_ON(PADDR_BITS > BITS_PER_LONG);
>
> Based on the above, I think this wants to be "(PADDR_BITS -
> PAGE_SHIFT) >= BITS_PER_LONG)".
Ack
>
>> xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
>> printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
>> }
>
> Cheers,
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG
2022-12-01 12:12 ` Ayan Kumar Halder
@ 2022-12-01 17:37 ` Julien Grall
0 siblings, 0 replies; 5+ messages in thread
From: Julien Grall @ 2022-12-01 17:37 UTC (permalink / raw)
To: Ayan Kumar Halder, Ayan Kumar Halder, xen-devel
Cc: sstabellini, stefanos, andrew.cooper3, george.dunlap, jbeulich,
bobbyeshleman, alistair.francis, connojdavis, wl
On 01/12/2022 12:12, Ayan Kumar Halder wrote:
>
> On 01/12/2022 10:26, Julien Grall wrote:
>> Hi Ayan,
>
> Hi Julien,
>
> I have a question.
>
>>
>> On 01/12/2022 10:03, Ayan Kumar Halder wrote:
>>> It is possible for a pointer to represent physical memory of the same
>>> size.
>>> In other words, a 32 bit pointer can represent 32 bit addressable
>>> physical
>>> memory.
>>> Thus, issue a compilation failure only when the count of physical
>>> address bits
>>> is greater than BITS_PER_LONG (ie count of bits in void*).
>>
>> I am having difficult to understand how this description is related to
>> the BUILD_BUG_ON(). AFAIU, it is used to check that xenheap_bits can
>> be used in shift.
>>
>> If the unsigned long is 32-bit, then a shift of 32 could be undefined.
>> Looking at the current use, the shift are used with "xenheap_bits -
>> PAGE_SHIFT". So as long as PAGE_SHIFT is not 0, you would be fine.
> Ack
>>
>>>
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> ---
>>>
>>> Currently this change will not have any impact on the existing
>>> architectures.
>>> The following table illustrates PADDR_BITS vs BITS_PER_LONG of
>>> different archs
>>>
>>> ------------------------------------------------
>>> | Arch | PADDR_BITS | BITS_PER_LONG |
>>> ------------------------------------------------
>>> | Arm_64 | 48 | 64 |
>>> | Arm_32 | 40 | 32 |
>>> | RISCV_64 | Don't know | 64 |
>>> | x86 | 52 | 64 |
>>> -------------------------------------------------
>>
>> The Arm_32 line is a bit confusing because one would wonder why we
>> haven't seen this issue yet. So I think you want to clarify that the
>> code path is not used by Arm32.
>
> Do you want this clarification and the above/below explanation to be a
> part of the commit message ?
No I don't think this will be relevant in the final commit message.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-01 17:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-01 10:03 [XEN v1] Xen: Enable compilation when PADDR_BITS == BITS_PER_LONG Ayan Kumar Halder
2022-12-01 10:26 ` Julien Grall
2022-12-01 11:44 ` Jan Beulich
2022-12-01 12:12 ` Ayan Kumar Halder
2022-12-01 17:37 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.