Thanks for Ron's suggestions.
Hi Ard, Mark,
>
> Is there some feeling here that it would be ok to restrict this discussion to risc-v, and not bring in ARM considerations. WDYT?
>
Hi Ard, Mark,
Now the coreboot we are using does not support EFI and only supports
one interface DTB. It seems that we have to pass the firmware
information through DTB.
From another point of view, ACPI and SMBIOS are common modules of the
kernel, not only EFI, but also other interfaces can also be connected
to this module, such as 0xF0000 for SMBIOS,
CONFIG_ACPI_LEGACY_TABLES_LOOKUP for ACPI, this patch is also.
We just use the DTB channel to add a few nodes to complete the
transfer of firmware information, which does not interfere with DTS
itself.
We think it is unnecessary to add an ACPI-supporting framework under
the fdt framework we discussed before. We only need one set of ACPI
framework, but one more set will cause unnecessary trouble.
So, let's move on to this patch, shall we?
Thanks,
Yunhui
On Wed, 21 Jun 2023 at 10:04, 运辉崔 <[email protected]> wrote:
>
> Thanks for Ron's suggestions.
>
> Hi Ard, Mark,
> >
> > Is there some feeling here that it would be ok to restrict this discussion to risc-v, and not bring in ARM considerations. WDYT?
> >
>
> Hi Ard, Mark,
>
> Now the coreboot we are using does not support EFI and only supports
> one interface DTB. It seems that we have to pass the firmware
> information through DTB.
>
> From another point of view, ACPI and SMBIOS are common modules of the
> kernel, not only EFI, but also other interfaces can also be connected
> to this module, such as 0xF0000 for SMBIOS,
> CONFIG_ACPI_LEGACY_TABLES_LOOKUP for ACPI, this patch is also.
>
> We just use the DTB channel to add a few nodes to complete the
> transfer of firmware information, which does not interfere with DTS
> itself.
>
> We think it is unnecessary to add an ACPI-supporting framework under
> the fdt framework we discussed before. We only need one set of ACPI
> framework, but one more set will cause unnecessary trouble.
>
> So, let's move on to this patch, shall we?
>
How do you intend to provide the ACPI core with the memory attribute
information that it needs to access SystemMemory OpRegions?
Hi Ard,
On Sat, Jun 24, 2023 at 8:52 PM Ard Biesheuvel <[email protected]> wrote:
>
> How do you intend to provide the ACPI core with the memory attribute
> information that it needs to access SystemMemory OpRegions?
Regarding memory segments and attributes, our solution does not need
to build a memmap table in coreboot like EFI to connect to linux ACPI
core.
Because the memory segment and attributes have been passed through the
"memory" node and "reserved-memory" node attributes of DTS.
For Linux, no matter what kind of memory attributes of the firmware,
it is ultimately connected to the memblock module.
So the memory attributes you consider can be done through the existing
DTS (like Ron said before, Chrombook does everything through DTS).
So can we come to a consensus? Then start reviewing the code?
Thanks,
Yunhui
On Sun, 25 Jun 2023 at 09:33, 运辉崔 <[email protected]> wrote:
>
> Hi Ard,
>
> On Sat, Jun 24, 2023 at 8:52 PM Ard Biesheuvel <[email protected]> wrote:
> >
> > How do you intend to provide the ACPI core with the memory attribute
> > information that it needs to access SystemMemory OpRegions?
>
> Regarding memory segments and attributes, our solution does not need
> to build a memmap table in coreboot like EFI to connect to linux ACPI
> core.
>
So how do you expect the code below will behave?
arch/arm64/kernel/acpi.c:
270:void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
271-{
272- efi_memory_desc_t *md, *region = NULL;
273- pgprot_t prot;
274-
275- if (WARN_ON_ONCE(!efi_enabled(EFI_MEMMAP)))
276- return NULL;
acpi_os_ioremap() is used by all ACPI core code that needs to map MMIO
regions or DRAM from AML code. AML does not pass memory type
attributes, so we have to consult the EFI memory map for these.
As I have explained to you multiple times, ACPI on arm64 is *broken*
without the EFI memory map.
> Because the memory segment and attributes have been passed through the
> "memory" node and "reserved-memory" node attributes of DTS.
>
> For Linux, no matter what kind of memory attributes of the firmware,
> it is ultimately connected to the memblock module.
>
Incorrect. We are talking about any physical region here, not just
DRAM. And some DRAM regions may not be covered by memblock.
> So the memory attributes you consider can be done through the existing
> DTS (like Ron said before, Chrombook does everything through DTS).
>
> So can we come to a consensus? Then start reviewing the code?
>
No, sorry. Please try to understand the objections that I am raising
first. I am not saying this to annoy you, I am saying this because
your approach is flawed.
Hi Ard,
On Sun, Jun 25, 2023 at 3:43 PM Ard Biesheuvel <[email protected]> wrote:
>
> acpi_os_ioremap() is used by all ACPI core code that needs to map MMIO
> regions or DRAM from AML code. AML does not pass memory type
> attributes, so we have to consult the EFI memory map for these.
>
> As I have explained to you multiple times, ACPI on arm64 is *broken*
> without the EFI memory map.
>
As Ron's suggested:
"...
It would be nice to separate those pieces on RISC-V; certainly they
were separate for a very long time in the x86 world (we had ACPI+SMM
on coreboot laptops without UEFI for example)
...
"
If it cannot be solved temporarily on arm64, then we cannot let it
continue to be bound in RISC-V.
And on the linux-next branch, RISC-V arch is not bound to EFI.
void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
{
return memremap(phys, size, MEMREMAP_WB);
}
>
> Incorrect. We are talking about any physical region here, not just
> DRAM. And some DRAM regions may not be covered by memblock.
>
It is very strange that so many devices can complete the hardware
description through DTS without the problem you mentioned.
Even if there is, then it shouldn't be the problem that this patch
should solve, should it?
> No, sorry. Please try to understand the objections that I am raising
> first. I am not saying this to annoy you, I am saying this because
> your approach is flawed.
The implementation is right in front of us, we need to support ACPI on
RISC-V based on coreboot.
Thanks,
Yunhui
On Sun, 25 Jun 2023 at 13:54, 运辉崔 <[email protected]> wrote:
>
> Hi Ard,
>
> On Sun, Jun 25, 2023 at 3:43 PM Ard Biesheuvel <[email protected]> wrote:
> >
> > acpi_os_ioremap() is used by all ACPI core code that needs to map MMIO
> > regions or DRAM from AML code. AML does not pass memory type
> > attributes, so we have to consult the EFI memory map for these.
> >
> > As I have explained to you multiple times, ACPI on arm64 is *broken*
> > without the EFI memory map.
> >
>
> As Ron's suggested:
> "...
> It would be nice to separate those pieces on RISC-V; certainly they
> were separate for a very long time in the x86 world (we had ACPI+SMM
> on coreboot laptops without UEFI for example)
> ...
> "
>
> If it cannot be solved temporarily on arm64, then we cannot let it
> continue to be bound in RISC-V.
> And on the linux-next branch, RISC-V arch is not bound to EFI.
> void *acpi_os_ioremap(acpi_physical_address phys, acpi_size size)
> {
> return memremap(phys, size, MEMREMAP_WB);
> }
>
>
>
> >
> > Incorrect. We are talking about any physical region here, not just
> > DRAM. And some DRAM regions may not be covered by memblock.
> >
> It is very strange that so many devices can complete the hardware
> description through DTS without the problem you mentioned.
> Even if there is, then it shouldn't be the problem that this patch
> should solve, should it?
>
> > No, sorry. Please try to understand the objections that I am raising
> > first. I am not saying this to annoy you, I am saying this because
> > your approach is flawed.
>
> The implementation is right in front of us, we need to support ACPI on
> RISC-V based on coreboot.
>
If this is only used on RISC-V, and implemented under arch/riscv, I
have no objections.