Hi Roger,
On 11/08/2025 09:07, Roger Pau Monné wrote:
On Fri, Aug 08, 2025 at 06:21:29PM +0100, Julien Grall wrote:
Hi Roger,
On 05/08/2025 10:52, Roger Pau Monne wrote:
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index a77b31071ed8..ba35bf1fe3bb 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -256,9 +256,11 @@ void __init init_pdx(void)
{
const struct membanks *mem = bootinfo_get_mem();
paddr_t bank_start, bank_size, bank_end, ram_end = 0;
- int bank;
+ unsigned int bank;
#ifndef CONFIG_PDX_NONE
+ for ( bank = 0 ; bank < mem->nr_banks; bank++ )
+ pfn_pdx_add_region(mem->bank[bank].start, mem->bank[bank].size);
/*
* Arm does not have any restrictions on the bits to compress. Pass 0 to
* let the common code further restrict the mask.
@@ -266,26 +268,24 @@ void __init init_pdx(void)
* If the logic changes in pfn_pdx_hole_setup we might have to
* update this function too.
*/
- uint64_t mask = pdx_init_mask(0x0);
-
- for ( bank = 0 ; bank < mem->nr_banks; bank++ )
- {
- bank_start = mem->bank[bank].start;
- bank_size = mem->bank[bank].size;
-
- mask |= bank_start | pdx_region_mask(bank_start, bank_size);
- }
+ pfn_pdx_compression_setup(0);
for ( bank = 0 ; bank < mem->nr_banks; bank++ )
{
- bank_start = mem->bank[bank].start;
- bank_size = mem->bank[bank].size;
-
- if (~mask & pdx_region_mask(bank_start, bank_size))
- mask = 0;
+ if ( !pdx_is_region_compressible(
+ mem->bank[bank].start,
+ PFN_UP(mem->bank[bank].start + mem->bank[bank].size) -
+ PFN_DOWN(mem->bank[bank].start)) )
This code is a bit too verbose. Can we at least introduce "bank =
&mem->bank[bank]" to reduce a bit the verbosity?
I cannot introduce a `bank` local variable as it's already used as the
index cursor for the loop. Would you be fine with:
Ah yes. I am fine with the logic below. I am happy if you want to commit
message without resending the series.
Cheers,
--
Julien Grall