[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI table
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables |
Date: |
Fri, 24 Feb 2023 17:14:42 +0100 |
On Wed, 8 Feb 2023 09:06:48 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Feb 8, 2023 at 2:15 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Wed, Feb 08, 2023 at 12:10:55AM +0800, Bin Meng wrote:
> > > On Mon, Feb 6, 2023 at 9:24 PM Sunil V L <sunilvl@ventanamicro.com>
> > > wrote:
> > > >
> > > > On Mon, Feb 06, 2023 at 06:17:50PM +0800, Bin Meng wrote:
> > > > > On Thu, Feb 2, 2023 at 12:54 PM Sunil V L <sunilvl@ventanamicro.com>
> > > > > wrote:
> > > > > >
> > > > > > Add few basic ACPI tables and DSDT with few devices in a
> > > > > > new file virt-acpi-build.c.
> > > > > >
> > > > > > These are mostly leveraged from arm64.
> > > > >
> > > > > There are lots of same ACPI codes existing in x86/arm/riscv. I believe
> > > > > some refactoring work is needed before ACPI support fully lands on
> > > > > RISC-V.
> > > > > For example, we can extract the common part among x86/arm/riscv into a
> > > > > separate file, like hw/acpi/acpi-build.c?
> > > > >
> > > >
> > > > While it appears like there is same code in multiple places, those
> > > > functions take architecture specific MachineState parameter. Some tables
> > > > like MADT though with same name, will have different contents for
> > > > different architectures.
> > > >
> > > > Only one function which Daniel also pointed is the wrapper
> > > > acpi_align_size() which can be made common. I am not
> > > > sure whether it is worth of refactoring.
> > > >
> > >
> > > It's more than that. For example,
> > >
> > > With acpi_dsdt_add_cpus(), the only meaningful parameter is the number
> > > of cpus, so there is no need to pass the architecture specific
> > > MachineState as the parameter.
> > >
> > I would not make this function common. The reason is, an architecture may
> > choose to attach different ACPI objects under the CPU node.
> >
>
> Is it possible to insert architect specific CPU nodes after this
> common API builds the basic CPU node in DSDT?
What do you mean by "architect specific CPU", any examples?
>
> > > Likewise, acpi_dsdt_add_fw_cfg() can be made generic as well.
> > >
> > The issue is, these things are not exactly common across all architectures.
> > x86 has bit different data in these objects. While today it appears they
> > are same for riscv and arm, in future things may change for an architecture.
> > It doesn't look like it is a standard practice to build files under
> > hw/acpi for specific architectures. Hence, IMO, it is better to keep these
> > things in architecture specific folders to allow them to do differently in
> > future.
> >
>
> It looks like hw/acpi/ghes.c is for Arm from MAINTAINERS.
>
> > But I look forward for the feedback from other architecture maintainers on
> > this topic. My experience in qemu is very limited. So, I need help from
> > experts.
>
> + Michael and Igor
>
> Regards,
> Bin
>
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, (continued)
[PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Sunil V L, 2023/02/01
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Bin Meng, 2023/02/06
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Sunil V L, 2023/02/06
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Bin Meng, 2023/02/07
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Sunil V L, 2023/02/07
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Bin Meng, 2023/02/07
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables, Sunil V L, 2023/02/07
- Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables,
Igor Mammedov <=
[PATCH 05/10] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT, Sunil V L, 2023/02/01
[PATCH 06/10] hw/riscv/virt: virt-acpi-build.c: Add RHCT Table, Sunil V L, 2023/02/01
[PATCH 08/10] hw/riscv/Kconfig: virt: Enable ACPI config options, Sunil V L, 2023/02/01
[PATCH 07/10] hw/riscv: meson.build: Build virt-acpi-build.c, Sunil V L, 2023/02/01
[PATCH 09/10] hw/riscv/virt.c: Initialize the ACPI tables, Sunil V L, 2023/02/01