Re: [BUG] hw/i386/pc.c: CXL Fixed Memory Window should not reserve e820 in bios

[Ani Sinha]

+Gerd Hoffmann

On Tue, Oct 18, 2022 at 8:16 PM Gregory Price <gourry.memverge@gmail.com> wrote:
>
> This patch does not resolve the issue, reserved entries are still created.
>
> [    0.000000] BIOS-e820: [mem 0x0000000280000000-0x00000002800fffff] reserved
> [    0.000000] BIOS-e820: [mem 0x0000000290000000-0x000000029fffffff] reserved
>
> # cat /proc/iomem
> 290000000-29fffffff : CXL Window 0
>   290000000-29fffffff : Reserved
>
> # cxl create-region -m -d decoder0.0 -w 1 -g 256 mem0
> cxl region: create_region: region0: set_size failed: Numerical result out of 
> range
> cxl region: cmd_create_region: created 0 regions
>
> On Tue, Oct 18, 2022 at 2:05 AM Ani Sinha <ani@anisinha.ca> wrote:
>>
>> On Tue, Oct 18, 2022 at 5:14 AM Gregory Price <gourry.memverge@gmail.com> 
>> wrote:
>> >
>> > Early-boot e820 records will be inserted by the bios/efi/early boot
>> > software and be reported to the kernel via insert_resource.  Later, when
>> > CXL drivers iterate through the regions again, they will insert another
>> > resource and make the RESERVED memory area a child.
>>
>> I have already sent a patch
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg882012.html .
>> When the patch is applied, there would not be any reserved entries
>> even with passing E820_RESERVED .
>> So this patch needs to be evaluated in the light of the above patch I
>> sent. Once you apply my patch, does the issue still exist?
>>
>> >
>> > This RESERVED memory area causes the memory region to become unusable,
>> > and as a result attempting to create memory regions with
>> >
>> >     `cxl create-region ...`
>> >
>> > Will fail due to the RESERVED area intersecting with the CXL window.
>> >
>> >
>> > During boot the following traceback is observed:
>> >
>> > 0xffffffff81101650 in insert_resource_expand_to_fit ()
>> > 0xffffffff83d964c5 in e820__reserve_resources_late ()
>> > 0xffffffff83e03210 in pcibios_resource_survey ()
>> > 0xffffffff83e04f4a in pcibios_init ()
>> >
>> > Which produces a call to reserve the CFMWS area:
>> >
>> > (gdb) p *new
>> > $54 = {start = 0x290000000, end = 0x2cfffffff, name = "Reserved",
>> >        flags = 0x200, desc = 0x7, parent = 0x0, sibling = 0x0,
>> >        child = 0x0}
>> >
>> > Later the Kernel parses ACPI tables and reserves the exact same area as
>> > the CXL Fixed Memory Window.  The use of `insert_resource_conflict`
>> > retains the RESERVED region and makes it a child of the new region.
>> >
>> > 0xffffffff811016a4 in insert_resource_conflict ()
>> >                       insert_resource ()
>> > 0xffffffff81a81389 in cxl_parse_cfmws ()
>> > 0xffffffff818c4a81 in call_handler ()
>> >                       acpi_parse_entries_array ()
>> >
>> > (gdb) p/x *new
>> > $59 = {start = 0x290000000, end = 0x2cfffffff, name = "CXL Window 0",
>> >        flags = 0x200, desc = 0x0, parent = 0x0, sibling = 0x0,
>> >        child = 0x0}
>> >
>> > This produces the following output in /proc/iomem:
>> >
>> > 590000000-68fffffff : CXL Window 0
>> >   590000000-68fffffff : Reserved
>> >
>> > This reserved area causes `get_free_mem_region()` to fail due to a check
>> > against `__region_intersects()`.  Due to this reserved area, the
>> > intersect check will only ever return REGION_INTERSECTS, which causes
>> > `cxl create-region` to always fail.
>> >
>> > Signed-off-by: Gregory Price <gregory.price@memverge.com>
>> > ---
>> >  hw/i386/pc.c | 2 --
>> >  1 file changed, 2 deletions(-)
>> >
>> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> > index 566accf7e6..5bf5465a21 100644
>> > --- a/hw/i386/pc.c
>> > +++ b/hw/i386/pc.c
>> > @@ -1061,7 +1061,6 @@ void pc_memory_init(PCMachineState *pcms,
>> >          hwaddr cxl_size = MiB;
>> >
>> >          cxl_base = pc_get_cxl_range_start(pcms);
>> > -        e820_add_entry(cxl_base, cxl_size, E820_RESERVED);
>> >          memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
>> >          memory_region_add_subregion(system_memory, cxl_base, mr);
>> >          cxl_resv_end = cxl_base + cxl_size;
>> > @@ -1077,7 +1076,6 @@ void pc_memory_init(PCMachineState *pcms,
>> >                  memory_region_init_io(&fw->mr, OBJECT(machine), 
>> > &cfmws_ops, fw,
>> >                                        "cxl-fixed-memory-region", 
>> > fw->size);
>> >                  memory_region_add_subregion(system_memory, fw->base, 
>> > &fw->mr);
>> > -                e820_add_entry(fw->base, fw->size, E820_RESERVED);
>> >                  cxl_fmw_base += fw->size;
>> >                  cxl_resv_end = cxl_fmw_base;
>> >              }
>> > --
>> > 2.37.3
>> >