[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: |
Sunil V L |
Subject: |
Re: [PATCH 04/10] hw/riscv/virt: virt-acpi-build.c: Add basic ACPI tables |
Date: |
Tue, 7 Feb 2023 23:45:25 +0530 |
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.
> 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.
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.
Thanks!
Sunil
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, (continued)
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Andrea Bolognani, 2023/02/07
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Thomas Huth, 2023/02/07
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Andrea Bolognani, 2023/02/07
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Andrew Jones, 2023/02/07
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Andrea Bolognani, 2023/02/08
- Re: [PATCH 02/10] hw/riscv/virt: Add a switch to enable/disable ACPI, Thomas Huth, 2023/02/09
[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 <=
- 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, 2023/02/24
[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