On Wed, Feb 8, 2023 at 2:15 AM Sunil V L <suni...@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 <suni...@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 <suni...@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? > > 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