[PATCH 00/10] RISC-V: ACPI: Enable AIA and update RHC

[PATCH 00/10] RISC-V: ACPI: Enable AIA and update RHC

[Sunil V L]

This series primarily enables AIA interrupt controllers in
ACPI tables for RISC-V virt platform. It also updates RHCT
with CMO and MMU related information.

Below ECRs for these changes are approved by ASWG and will be
available in next ACPI spec release.

1) MADT (AIA) - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
2) RHCT - https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view?usp=sharing

First patch in this series is to migrate a couple of functions from
ARM architecture to common code so that RISC-V doesn't need to
duplicate the same.

The patch set is based on Alistair's riscv-to-apply.next branch.

These changes are also available in  riscv_acpi_b2_v1 branch at:
https://github.com/vlsunil/qemu/

Sunil V L (10):
  hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location
  hw/riscv: virt: Add PCI bus reference in RISCVVirtState
  hw/riscv: virt: Make few IMSIC macros and functions public
  hw/riscv: virt: Add PCIe HIGHMEM in memmap
  hw/riscv/virt-acpi-build.c: Add AIA support in RINTC
  hw/riscv/virt-acpi-build.c: Add IMSIC in the MADT
  hw/riscv/virt-acpi-build.c: Add APLIC in the MADT
  hw/riscv/virt-acpi-build.c: Add CMO information in RHCT
  hw/riscv/virt-acpi-build.c: Add MMU node in RHCT
  hw/riscv/virt-acpi-build.c: Add IO controllers and devices

 hw/acpi/aml-build.c         |  41 +++++
 hw/arm/virt-acpi-build.c    |  42 -----
 hw/riscv/Kconfig            |   1 +
 hw/riscv/virt-acpi-build.c  | 321 +++++++++++++++++++++++++++++++++---
 hw/riscv/virt.c             |  62 ++++---
 include/hw/acpi/aml-build.h |   6 +
 include/hw/riscv/virt.h     |  35 +++-
 7 files changed, 416 insertions(+), 92 deletions(-)

-- 
2.39.2

[PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Sunil V L]

The functions which add fw_cfg and virtio to DSDT are same for ARM
and RISC-V. So, instead of duplicating in RISC-V, move them from
hw/arm/virt-acpi-build.c to common aml-build.c.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    | 42 -------------------------------------
 hw/riscv/virt-acpi-build.c  | 16 --------------
 include/hw/acpi/aml-build.h |  6 ++++++
 4 files changed, 47 insertions(+), 58 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ea331a20d1..eeb1263c8c 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source)
 
     return var;
 }
+
+void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
+{
+    Aml *dev = aml_device("FWCF");
+    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
+    /* device present, functioning, decoding, not shown in UI */
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
+                                       fw_cfg_memmap->size, AML_READ_WRITE));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
+void acpi_dsdt_add_virtio(Aml *scope,
+                          const MemMapEntry *virtio_mmio_memmap,
+                          uint32_t mmio_irq, int num)
+{
+    hwaddr base = virtio_mmio_memmap->base;
+    hwaddr size = virtio_mmio_memmap->size;
+    int i;
+
+    for (i = 0; i < num; i++) {
+        uint32_t irq = mmio_irq + i;
+        Aml *dev = aml_device("VR%02u", i);
+        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
+        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
+        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+        Aml *crs = aml_resource_template();
+        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
+        aml_append(crs,
+                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                                 AML_EXCLUSIVE, &irq, 1));
+        aml_append(dev, aml_name_decl("_CRS", crs));
+        aml_append(scope, dev);
+        base += size;
+    }
+}
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6b674231c2..fdedb68e2b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -35,7 +35,6 @@
 #include "target/arm/cpu.h"
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/acpi.h"
-#include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/acpi/aml-build.h"
 #include "hw/acpi/utils.h"
@@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
     aml_append(scope, dev);
 }
 
-static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
-{
-    Aml *dev = aml_device("FWCF");
-    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
-    /* device present, functioning, decoding, not shown in UI */
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
-    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
-
-    Aml *crs = aml_resource_template();
-    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
-                                       fw_cfg_memmap->size, AML_READ_WRITE));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-    aml_append(scope, dev);
-}
-
 static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
 {
     Aml *dev, *crs;
@@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
     aml_append(scope, dev);
 }
 
-static void acpi_dsdt_add_virtio(Aml *scope,
-                                 const MemMapEntry *virtio_mmio_memmap,
-                                 uint32_t mmio_irq, int num)
-{
-    hwaddr base = virtio_mmio_memmap->base;
-    hwaddr size = virtio_mmio_memmap->size;
-    int i;
-
-    for (i = 0; i < num; i++) {
-        uint32_t irq = mmio_irq + i;
-        Aml *dev = aml_device("VR%02u", i);
-        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
-        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
-        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
-
-        Aml *crs = aml_resource_template();
-        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
-        aml_append(crs,
-                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                                 AML_EXCLUSIVE, &irq, 1));
-        aml_append(dev, aml_name_decl("_CRS", crs));
-        aml_append(scope, dev);
-        base += size;
-    }
-}
-
 static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
                               uint32_t irq, VirtMachineState *vms)
 {
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 7331248f59..01843e4509 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
     }
 }
 
-static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
-{
-    Aml *dev = aml_device("FWCF");
-    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
-
-    /* device present, functioning, decoding, not shown in UI */
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
-    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
-
-    Aml *crs = aml_resource_template();
-    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
-                                       fw_cfg_memmap->size, AML_READ_WRITE));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-    aml_append(scope, dev);
-}
-
 /* RHCT Node[N] starts at offset 56 */
 #define RHCT_NODE_ARRAY_OFFSET 56
 
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d1fb08514b..c4a8967310 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -3,6 +3,7 @@
 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/bios-linker-loader.h"
+#include "hw/nvram/fw_cfg.h"
 
 #define ACPI_BUILD_APPNAME6 "BOCHS "
 #define ACPI_BUILD_APPNAME8 "BXPC    "
@@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
 
 void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
                 const char *oem_id, const char *oem_table_id);
+
+void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap);
+void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
+                          uint32_t mmio_irq, int num);
+
 #endif
-- 
2.39.2

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> The functions which add fw_cfg and virtio to DSDT are same for ARM
> and RISC-V. So, instead of duplicating in RISC-V, move them from
> hw/arm/virt-acpi-build.c to common aml-build.c.

Nice.

> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
>   hw/arm/virt-acpi-build.c    | 42 -------------------------------------
>   hw/riscv/virt-acpi-build.c  | 16 --------------
>   include/hw/acpi/aml-build.h |  6 ++++++
>   4 files changed, 47 insertions(+), 58 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea331a20d1..eeb1263c8c 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source)
>   
>       return var;
>   }
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> +{
> +    Aml *dev = aml_device("FWCF");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> +    /* device present, functioning, decoding, not shown in UI */
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> +                                       fw_cfg_memmap->size, AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +void acpi_dsdt_add_virtio(Aml *scope,
> +                          const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num)
> +{
> +    hwaddr base = virtio_mmio_memmap->base;
> +    hwaddr size = virtio_mmio_memmap->size;
> +    int i;
> +
> +    for (i = 0; i < num; i++) {
> +        uint32_t irq = mmio_irq + i;
> +        Aml *dev = aml_device("VR%02u", i);
> +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +        Aml *crs = aml_resource_template();
> +        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> +        aml_append(crs,
> +                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                                 AML_EXCLUSIVE, &irq, 1));
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +        aml_append(scope, dev);
> +        base += size;
> +    }
> +}
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6b674231c2..fdedb68e2b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -35,7 +35,6 @@
>   #include "target/arm/cpu.h"
>   #include "hw/acpi/acpi-defs.h"
>   #include "hw/acpi/acpi.h"
> -#include "hw/nvram/fw_cfg.h"
>   #include "hw/acpi/bios-linker-loader.h"
>   #include "hw/acpi/aml-build.h"
>   #include "hw/acpi/utils.h"
> @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>       aml_append(scope, dev);
>   }
>   
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>   static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>   {
>       Aml *dev, *crs;
> @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>       aml_append(scope, dev);
>   }
>   
> -static void acpi_dsdt_add_virtio(Aml *scope,
> -                                 const MemMapEntry *virtio_mmio_memmap,
> -                                 uint32_t mmio_irq, int num)
> -{
> -    hwaddr base = virtio_mmio_memmap->base;
> -    hwaddr size = virtio_mmio_memmap->size;
> -    int i;
> -
> -    for (i = 0; i < num; i++) {
> -        uint32_t irq = mmio_irq + i;
> -        Aml *dev = aml_device("VR%02u", i);
> -        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> -        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -        Aml *crs = aml_resource_template();
> -        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> -        aml_append(crs,
> -                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, &irq, 1));
> -        aml_append(dev, aml_name_decl("_CRS", crs));
> -        aml_append(scope, dev);
> -        base += size;
> -    }
> -}
> -
>   static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>                                 uint32_t irq, VirtMachineState *vms)
>   {
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 7331248f59..01843e4509 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>       }
>   }
>   
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>   /* RHCT Node[N] starts at offset 56 */
>   #define RHCT_NODE_ARRAY_OFFSET 56
>   
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d1fb08514b..c4a8967310 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -3,6 +3,7 @@
>   
>   #include "hw/acpi/acpi-defs.h"
>   #include "hw/acpi/bios-linker-loader.h"
> +#include "hw/nvram/fw_cfg.h"
>   
>   #define ACPI_BUILD_APPNAME6 "BOCHS "
>   #define ACPI_BUILD_APPNAME8 "BXPC    "
> @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>   
>   void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>                   const char *oem_id, const char *oem_table_id);
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap);
> +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num);
> +
>   #endif

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Alistair Francis]

On Thu, Jul 13, 2023 at 2:41 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> The functions which add fw_cfg and virtio to DSDT are same for ARM
> and RISC-V. So, instead of duplicating in RISC-V, move them from
> hw/arm/virt-acpi-build.c to common aml-build.c.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
>  hw/arm/virt-acpi-build.c    | 42 -------------------------------------
>  hw/riscv/virt-acpi-build.c  | 16 --------------
>  include/hw/acpi/aml-build.h |  6 ++++++
>  4 files changed, 47 insertions(+), 58 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ea331a20d1..eeb1263c8c 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source)
>
>      return var;
>  }
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> +{
> +    Aml *dev = aml_device("FWCF");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> +    /* device present, functioning, decoding, not shown in UI */
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> +                                       fw_cfg_memmap->size, AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +void acpi_dsdt_add_virtio(Aml *scope,
> +                          const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num)
> +{
> +    hwaddr base = virtio_mmio_memmap->base;
> +    hwaddr size = virtio_mmio_memmap->size;
> +    int i;
> +
> +    for (i = 0; i < num; i++) {
> +        uint32_t irq = mmio_irq + i;
> +        Aml *dev = aml_device("VR%02u", i);
> +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +        Aml *crs = aml_resource_template();
> +        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> +        aml_append(crs,
> +                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                                 AML_EXCLUSIVE, &irq, 1));
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +        aml_append(scope, dev);
> +        base += size;
> +    }
> +}
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6b674231c2..fdedb68e2b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -35,7 +35,6 @@
>  #include "target/arm/cpu.h"
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/acpi/acpi.h"
> -#include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/acpi/aml-build.h"
>  #include "hw/acpi/utils.h"
> @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>      aml_append(scope, dev);
>  }
>
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
> @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>      aml_append(scope, dev);
>  }
>
> -static void acpi_dsdt_add_virtio(Aml *scope,
> -                                 const MemMapEntry *virtio_mmio_memmap,
> -                                 uint32_t mmio_irq, int num)
> -{
> -    hwaddr base = virtio_mmio_memmap->base;
> -    hwaddr size = virtio_mmio_memmap->size;
> -    int i;
> -
> -    for (i = 0; i < num; i++) {
> -        uint32_t irq = mmio_irq + i;
> -        Aml *dev = aml_device("VR%02u", i);
> -        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> -        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -        Aml *crs = aml_resource_template();
> -        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> -        aml_append(crs,
> -                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, &irq, 1));
> -        aml_append(dev, aml_name_decl("_CRS", crs));
> -        aml_append(scope, dev);
> -        base += size;
> -    }
> -}
> -
>  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>                                uint32_t irq, VirtMachineState *vms)
>  {
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 7331248f59..01843e4509 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>      }
>  }
>
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  /* RHCT Node[N] starts at offset 56 */
>  #define RHCT_NODE_ARRAY_OFFSET 56
>
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d1fb08514b..c4a8967310 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -3,6 +3,7 @@
>
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/acpi/bios-linker-loader.h"
> +#include "hw/nvram/fw_cfg.h"
>
>  #define ACPI_BUILD_APPNAME6 "BOCHS "
>  #define ACPI_BUILD_APPNAME8 "BXPC    "
> @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>
>  void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>                  const char *oem_id, const char *oem_table_id);
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap);
> +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num);
> +
>  #endif
> --
> 2.39.2
>
>

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Igor Mammedov]

On Wed, 12 Jul 2023 22:09:34 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> The functions which add fw_cfg and virtio to DSDT are same for ARM
> and RISC-V. So, instead of duplicating in RISC-V, move them from
> hw/arm/virt-acpi-build.c to common aml-build.c.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
>  hw/arm/virt-acpi-build.c    | 42 -------------------------------------
>  hw/riscv/virt-acpi-build.c  | 16 --------------
>  include/hw/acpi/aml-build.h |  6 ++++++
>  4 files changed, 47 insertions(+), 58 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c

patch looks fine modulo,
I'd put these into respective device files instead of generic
aml-build.c which was intended for basic AML primitives
(it 's got polluted over time with device specific functions
but that's not the reason to continue doing that).

Also having those functions along with devices models
goes along with self enumerating ACPI devices (currently
it works for x86 PCI/ISA device but there is no reason
that it can't work with other types as well when
I get there)

> index ea331a20d1..eeb1263c8c 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source)
>  
>      return var;
>  }
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> +{
> +    Aml *dev = aml_device("FWCF");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> +    /* device present, functioning, decoding, not shown in UI */
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> +                                       fw_cfg_memmap->size, AML_READ_WRITE));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +void acpi_dsdt_add_virtio(Aml *scope,
> +                          const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num)
> +{
> +    hwaddr base = virtio_mmio_memmap->base;
> +    hwaddr size = virtio_mmio_memmap->size;
> +    int i;
> +
> +    for (i = 0; i < num; i++) {
> +        uint32_t irq = mmio_irq + i;
> +        Aml *dev = aml_device("VR%02u", i);
> +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +        Aml *crs = aml_resource_template();
> +        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> +        aml_append(crs,
> +                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                                 AML_EXCLUSIVE, &irq, 1));
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +        aml_append(scope, dev);
> +        base += size;
> +    }
> +}
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6b674231c2..fdedb68e2b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -35,7 +35,6 @@
>  #include "target/arm/cpu.h"
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/acpi/acpi.h"
> -#include "hw/nvram/fw_cfg.h"
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/acpi/aml-build.h"
>  #include "hw/acpi/utils.h"
> @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>      aml_append(scope, dev);
>  }
>  
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
> @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>      aml_append(scope, dev);
>  }
>  
> -static void acpi_dsdt_add_virtio(Aml *scope,
> -                                 const MemMapEntry *virtio_mmio_memmap,
> -                                 uint32_t mmio_irq, int num)
> -{
> -    hwaddr base = virtio_mmio_memmap->base;
> -    hwaddr size = virtio_mmio_memmap->size;
> -    int i;
> -
> -    for (i = 0; i < num; i++) {
> -        uint32_t irq = mmio_irq + i;
> -        Aml *dev = aml_device("VR%02u", i);
> -        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> -        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -        Aml *crs = aml_resource_template();
> -        aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> -        aml_append(crs,
> -                   aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                                 AML_EXCLUSIVE, &irq, 1));
> -        aml_append(dev, aml_name_decl("_CRS", crs));
> -        aml_append(scope, dev);
> -        base += size;
> -    }
> -}
> -
>  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>                                uint32_t irq, VirtMachineState *vms)
>  {
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 7331248f59..01843e4509 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>      }
>  }
>  
> -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
> -{
> -    Aml *dev = aml_device("FWCF");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> -
> -    /* device present, functioning, decoding, not shown in UI */
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> -    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
> -                                       fw_cfg_memmap->size, AML_READ_WRITE));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  /* RHCT Node[N] starts at offset 56 */
>  #define RHCT_NODE_ARRAY_OFFSET 56
>  
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index d1fb08514b..c4a8967310 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -3,6 +3,7 @@
>  
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/acpi/bios-linker-loader.h"
> +#include "hw/nvram/fw_cfg.h"
>  
>  #define ACPI_BUILD_APPNAME6 "BOCHS "
>  #define ACPI_BUILD_APPNAME8 "BXPC    "
> @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>  
>  void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog,
>                  const char *oem_id, const char *oem_table_id);
> +
> +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap);
> +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
> +                          uint32_t mmio_irq, int num);
> +
>  #endif

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Sunil V L]

On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote:
> On Wed, 12 Jul 2023 22:09:34 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > The functions which add fw_cfg and virtio to DSDT are same for ARM
> > and RISC-V. So, instead of duplicating in RISC-V, move them from
> > hw/arm/virt-acpi-build.c to common aml-build.c.
> > 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
> >  hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
> >  hw/arm/virt-acpi-build.c    | 42 -------------------------------------
> >  hw/riscv/virt-acpi-build.c  | 16 --------------
> >  include/hw/acpi/aml-build.h |  6 ++++++
> >  4 files changed, 47 insertions(+), 58 deletions(-)
> > 
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> 
> patch looks fine modulo,
> I'd put these into respective device files instead of generic
> aml-build.c which was intended for basic AML primitives
> (it 's got polluted over time with device specific functions
> but that's not the reason to continue doing that).
> 
> Also having those functions along with devices models
> goes along with self enumerating ACPI devices (currently
> it works for x86 PCI/ISA device but there is no reason
> that it can't work with other types as well when
> I get there)
> 
Thanks!, Igor. Let me add them to device specific files as per your
recommendation.

Thanks!
Sunil

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Igor Mammedov]

On Tue, 25 Jul 2023 22:20:36 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote:
> > On Wed, 12 Jul 2023 22:09:34 +0530
> > Sunil V L <sunilvl@ventanamicro.com> wrote:
> >   
> > > The functions which add fw_cfg and virtio to DSDT are same for ARM
> > > and RISC-V. So, instead of duplicating in RISC-V, move them from
> > > hw/arm/virt-acpi-build.c to common aml-build.c.
> > > 
> > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > ---
> > >  hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
> > >  hw/arm/virt-acpi-build.c    | 42 -------------------------------------
> > >  hw/riscv/virt-acpi-build.c  | 16 --------------
> > >  include/hw/acpi/aml-build.h |  6 ++++++
> > >  4 files changed, 47 insertions(+), 58 deletions(-)
> > > 
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c  
> > 
> > patch looks fine modulo,
> > I'd put these into respective device files instead of generic
> > aml-build.c which was intended for basic AML primitives
> > (it 's got polluted over time with device specific functions
> > but that's not the reason to continue doing that).
> > 
> > Also having those functions along with devices models
> > goes along with self enumerating ACPI devices (currently
> > it works for x86 PCI/ISA device but there is no reason
> > that it can't work with other types as well when
> > I get there)
> >   
> Thanks!, Igor. Let me add them to device specific files as per your
> recommendation.
just be careful and build test other targets (while disabling the rest)
at least no to regress them due to build deps. (I'd pick 2 with ACPI
support that use and not uses affected code) and 1 that  uses device
model but doesn't use ACPI at all (if such exists)

> 
> Thanks!
> Sunil
> 

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Daniel Henrique Barboza]

On 7/26/23 05:25, Igor Mammedov wrote:
> On Tue, 25 Jul 2023 22:20:36 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
>> On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote:
>>> On Wed, 12 Jul 2023 22:09:34 +0530
>>> Sunil V L <sunilvl@ventanamicro.com> wrote:
>>>    
>>>> The functions which add fw_cfg and virtio to DSDT are same for ARM
>>>> and RISC-V. So, instead of duplicating in RISC-V, move them from
>>>> hw/arm/virt-acpi-build.c to common aml-build.c.
>>>>
>>>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>>>> ---
>>>>   hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
>>>>   hw/arm/virt-acpi-build.c    | 42 -------------------------------------
>>>>   hw/riscv/virt-acpi-build.c  | 16 --------------
>>>>   include/hw/acpi/aml-build.h |  6 ++++++
>>>>   4 files changed, 47 insertions(+), 58 deletions(-)
>>>>
>>>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>>
>>> patch looks fine modulo,
>>> I'd put these into respective device files instead of generic
>>> aml-build.c which was intended for basic AML primitives
>>> (it 's got polluted over time with device specific functions
>>> but that's not the reason to continue doing that).
>>>
>>> Also having those functions along with devices models
>>> goes along with self enumerating ACPI devices (currently
>>> it works for x86 PCI/ISA device but there is no reason
>>> that it can't work with other types as well when
>>> I get there)
>>>    
>> Thanks!, Igor. Let me add them to device specific files as per your
>> recommendation.
> just be careful and build test other targets (while disabling the rest)
> at least no to regress them due to build deps. (I'd pick 2 with ACPI
> support that use and not uses affected code) and 1 that  uses device
> model but doesn't use ACPI at all (if such exists)

Sunil is already aware of it but I'll also mention here since it seems relevant
to Igor's point.


This patch breaks i386-softmmu build:


FAILED: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o
cc -m64 -mcx16 -Ilibqemu-i386-softmmu.fa.p -I. -I.. -Itarget/i386 -I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /home/danielhb/work/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/danielhb/work/qemu -iquote /home/danielhb/work/qemu/include -iquote /home/danielhb/work/qemu/host/include/x86_64 -iquote /home/danielhb/work/qemu/host/include/generic -iquote /home/danielhb/work/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="i386-softmmu-config-target.h"' '-DCONFIG_DEVICES="i386-softmmu-config-devices.h"' -MD -MQ libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -MF libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o.d -o libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -c ../hw/i386/acpi-microvm.c
../hw/i386/acpi-microvm.c:48:13: error: conflicting types for ‘acpi_dsdt_add_virtio’; have ‘void(Aml *, MicrovmMachineState *)’
    48 | static void acpi_dsdt_add_virtio(Aml *scope,
       |             ^~~~~~~~~~~~~~~~~~~~
In file included from /home/danielhb/work/qemu/include/hw/acpi/acpi_aml_interface.h:5,
                  from ../hw/i386/acpi-microvm.c:29:
/home/danielhb/work/qemu/include/hw/acpi/aml-build.h:503:6: note: previous declaration of ‘acpi_dsdt_add_virtio’ with type ‘void(Aml *, const MemMapEntry *, uint32_t,  int)’ {aka ‘void(Aml *, const MemMapEntry *, unsigned int,  int)’}
   503 | void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
       |      ^~~~~~~~~~~~~~~~~~~~
[5/714] Compiling C object libqemu-i386-softmmu.fa.p/hw_i386_kvm_clock.c.o

This happens because the common 'acpi_dsdt_add_virtio' function matches a local
function with the same name in hw/i386/acpi-microvm.c. We would need to either
rename the shared helper or rename the local acpi-microvm function or do something
like Igor mentioned to avoid this name collision.


Thanks,

Daniel








> 
>>
>> Thanks!
>> Sunil
>>
> 

Re: [PATCH 01/10] hw/arm/virt-acpi-build.c: Move fw_cfg and virtio to common location

[Sunil V L]

On Wed, Aug 16, 2023 at 03:51:58PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 7/26/23 05:25, Igor Mammedov wrote:
> > On Tue, 25 Jul 2023 22:20:36 +0530
> > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > 
> > > On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote:
> > > > On Wed, 12 Jul 2023 22:09:34 +0530
> > > > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > > > The functions which add fw_cfg and virtio to DSDT are same for ARM
> > > > > and RISC-V. So, instead of duplicating in RISC-V, move them from
> > > > > hw/arm/virt-acpi-build.c to common aml-build.c.
> > > > > 
> > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > > > > ---
> > > > >   hw/acpi/aml-build.c         | 41 ++++++++++++++++++++++++++++++++++++
> > > > >   hw/arm/virt-acpi-build.c    | 42 -------------------------------------
> > > > >   hw/riscv/virt-acpi-build.c  | 16 --------------
> > > > >   include/hw/acpi/aml-build.h |  6 ++++++
> > > > >   4 files changed, 47 insertions(+), 58 deletions(-)
> > > > > 
> > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > 
> > > > patch looks fine modulo,
> > > > I'd put these into respective device files instead of generic
> > > > aml-build.c which was intended for basic AML primitives
> > > > (it 's got polluted over time with device specific functions
> > > > but that's not the reason to continue doing that).
> > > > 
> > > > Also having those functions along with devices models
> > > > goes along with self enumerating ACPI devices (currently
> > > > it works for x86 PCI/ISA device but there is no reason
> > > > that it can't work with other types as well when
> > > > I get there)
> > > Thanks!, Igor. Let me add them to device specific files as per your
> > > recommendation.
> > just be careful and build test other targets (while disabling the rest)
> > at least no to regress them due to build deps. (I'd pick 2 with ACPI
> > support that use and not uses affected code) and 1 that  uses device
> > model but doesn't use ACPI at all (if such exists)
> 
> Sunil is already aware of it but I'll also mention here since it seems relevant
> to Igor's point.
> 
Thanks! Daniel. Yes, I am aware of the issue and will fix it along with
Igor's suggestion. I need to fix this irrespective of the approach.

Thanks,
Sunil
> 
> This patch breaks i386-softmmu build:
> 
> 
> FAILED: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o
> cc -m64 -mcx16 -Ilibqemu-i386-softmmu.fa.p -I. -I.. -Itarget/i386 -I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /home/danielhb/work/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/danielhb/work/qemu -iquote /home/danielhb/work/qemu/include -iquote /home/danielhb/work/qemu/host/include/x86_64 -iquote /home/danielhb/work/qemu/host/include/generic -iquote /home/danielhb/work/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="i386-softmmu-config-target.h"' '-DCONFIG_DEVICES="i386-softmmu-config-devices.h"' -MD -MQ libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -MF libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o.d -o libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -c ../hw/i386/acpi-microvm.c
> ../hw/i386/acpi-microvm.c:48:13: error: conflicting types for ‘acpi_dsdt_add_virtio’; have ‘void(Aml *, MicrovmMachineState *)’
>    48 | static void acpi_dsdt_add_virtio(Aml *scope,
>       |             ^~~~~~~~~~~~~~~~~~~~
> In file included from /home/danielhb/work/qemu/include/hw/acpi/acpi_aml_interface.h:5,
>                  from ../hw/i386/acpi-microvm.c:29:
> /home/danielhb/work/qemu/include/hw/acpi/aml-build.h:503:6: note: previous declaration of ‘acpi_dsdt_add_virtio’ with type ‘void(Aml *, const MemMapEntry *, uint32_t,  int)’ {aka ‘void(Aml *, const MemMapEntry *, unsigned int,  int)’}
>   503 | void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap,
>       |      ^~~~~~~~~~~~~~~~~~~~
> [5/714] Compiling C object libqemu-i386-softmmu.fa.p/hw_i386_kvm_clock.c.o
> 
> This happens because the common 'acpi_dsdt_add_virtio' function matches a local
> function with the same name in hw/i386/acpi-microvm.c. We would need to either
> rename the shared helper or rename the local acpi-microvm function or do something
> like Igor mentioned to avoid this name collision.
> 
> 
> Thanks,
> 
> Daniel
> 
> 
> 
> 
> 
> 
> 
> 
> > 
> > > 
> > > Thanks!
> > > Sunil
> > > 
> > 

[PATCH 02/10] hw/riscv: virt: Add PCI bus reference in RISCVVirtState

[Sunil V L]

The PCI bus information is needed in RISCVVirtState so that other
files like virt-acpi-build.c can make use of it. Add new field in
RISCVVirtState so that ACPI code can use it.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt.c         | 6 ++++--
 include/hw/riscv/virt.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d90286dc46..46d3341113 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1073,7 +1073,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
                                           hwaddr high_mmio_base,
                                           hwaddr high_mmio_size,
                                           hwaddr pio_base,
-                                          DeviceState *irqchip)
+                                          DeviceState *irqchip,
+                                          RISCVVirtState *s)
 {
     DeviceState *dev;
     MemoryRegion *ecam_alias, *ecam_reg;
@@ -1113,6 +1114,7 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
         gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ + i);
     }
 
+    s->bus = PCI_HOST_BRIDGE(dev)->bus;
     return dev;
 }
 
@@ -1502,7 +1504,7 @@ static void virt_machine_init(MachineState *machine)
                    virt_high_pcie_memmap.base,
                    virt_high_pcie_memmap.size,
                    memmap[VIRT_PCIE_PIO].base,
-                   pcie_irqchip);
+                   pcie_irqchip, s);
 
     create_platform_bus(s, mmio_irqchip);
 
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index e5c474b26e..4ef1f660ab 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -60,6 +60,7 @@ struct RISCVVirtState {
     char *oem_table_id;
     OnOffAuto acpi;
     const MemMapEntry *memmap;
+    PCIBus *bus;
 };
 
 enum {
-- 
2.39.2

Re: [PATCH 02/10] hw/riscv: virt: Add PCI bus reference in RISCVVirtState

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> The PCI bus information is needed in RISCVVirtState so that other
> files like virt-acpi-build.c can make use of it. Add new field in
> RISCVVirtState so that ACPI code can use it.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>   hw/riscv/virt.c         | 6 ++++--
>   include/hw/riscv/virt.h | 1 +
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d90286dc46..46d3341113 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1073,7 +1073,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>                                             hwaddr high_mmio_base,
>                                             hwaddr high_mmio_size,
>                                             hwaddr pio_base,
> -                                          DeviceState *irqchip)
> +                                          DeviceState *irqchip,
> +                                          RISCVVirtState *s)
>   {
>       DeviceState *dev;
>       MemoryRegion *ecam_alias, *ecam_reg;
> @@ -1113,6 +1114,7 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>           gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ + i);
>       }
>   
> +    s->bus = PCI_HOST_BRIDGE(dev)->bus;
>       return dev;
>   }
>   
> @@ -1502,7 +1504,7 @@ static void virt_machine_init(MachineState *machine)
>                      virt_high_pcie_memmap.base,
>                      virt_high_pcie_memmap.size,
>                      memmap[VIRT_PCIE_PIO].base,
> -                   pcie_irqchip);
> +                   pcie_irqchip, s);

I wonder whether we could use 's' inside gpex_pcie_init() to avoid passing all
this memmap stuff to the function. It seems that most, if not all these values,
can be derived from s->memmap[]. A work for another day perhaps.


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   
>       create_platform_bus(s, mmio_irqchip);
>   
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index e5c474b26e..4ef1f660ab 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -60,6 +60,7 @@ struct RISCVVirtState {
>       char *oem_table_id;
>       OnOffAuto acpi;
>       const MemMapEntry *memmap;
> +    PCIBus *bus;
>   };
>   
>   enum {

Re: [PATCH 02/10] hw/riscv: virt: Add PCI bus reference in RISCVVirtState

[Alistair Francis]

On Thu, Jul 13, 2023 at 2:42 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> The PCI bus information is needed in RISCVVirtState so that other
> files like virt-acpi-build.c can make use of it. Add new field in
> RISCVVirtState so that ACPI code can use it.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c         | 6 ++++--
>  include/hw/riscv/virt.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d90286dc46..46d3341113 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1073,7 +1073,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>                                            hwaddr high_mmio_base,
>                                            hwaddr high_mmio_size,
>                                            hwaddr pio_base,
> -                                          DeviceState *irqchip)
> +                                          DeviceState *irqchip,
> +                                          RISCVVirtState *s)
>  {
>      DeviceState *dev;
>      MemoryRegion *ecam_alias, *ecam_reg;
> @@ -1113,6 +1114,7 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>          gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ + i);
>      }
>
> +    s->bus = PCI_HOST_BRIDGE(dev)->bus;
>      return dev;
>  }
>
> @@ -1502,7 +1504,7 @@ static void virt_machine_init(MachineState *machine)
>                     virt_high_pcie_memmap.base,
>                     virt_high_pcie_memmap.size,
>                     memmap[VIRT_PCIE_PIO].base,
> -                   pcie_irqchip);
> +                   pcie_irqchip, s);
>
>      create_platform_bus(s, mmio_irqchip);
>
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index e5c474b26e..4ef1f660ab 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -60,6 +60,7 @@ struct RISCVVirtState {
>      char *oem_table_id;
>      OnOffAuto acpi;
>      const MemMapEntry *memmap;
> +    PCIBus *bus;
>  };
>
>  enum {
> --
> 2.39.2
>
>

[PATCH 03/10] hw/riscv: virt: Make few IMSIC macros and functions public

[Sunil V L]

Some macros and static function related to IMSIC are defined
in virt.c. They are required in virt-acpi-build.c. So, make them
public.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt.c         | 25 +------------------------
 include/hw/riscv/virt.h | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 46d3341113..f6067db8ec 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -37,7 +37,6 @@
 #include "hw/riscv/numa.h"
 #include "hw/intc/riscv_aclint.h"
 #include "hw/intc/riscv_aplic.h"
-#include "hw/intc/riscv_imsic.h"
 #include "hw/intc/sifive_plic.h"
 #include "hw/misc/sifive_test.h"
 #include "hw/platform-bus.h"
@@ -53,28 +52,6 @@
 #include "hw/acpi/aml-build.h"
 #include "qapi/qapi-visit-common.h"
 
-/*
- * The virt machine physical address space used by some of the devices
- * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
- * number of CPUs, and number of IMSIC guest files.
- *
- * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
- * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
- * of virt machine physical address space.
- */
-
-#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
-#if VIRT_IMSIC_GROUP_MAX_SIZE < \
-    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
-#error "Can't accomodate single IMSIC group in address space"
-#endif
-
-#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
-                                        VIRT_IMSIC_GROUP_MAX_SIZE)
-#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
-#error "Can't accomodate all IMSIC groups in address space"
-#endif
-
 static const MemMapEntry virt_memmap[] = {
     [VIRT_DEBUG] =        {        0x0,         0x100 },
     [VIRT_MROM] =         {     0x1000,        0xf000 },
@@ -505,7 +482,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     g_free(plic_cells);
 }
 
-static uint32_t imsic_num_bits(uint32_t count)
+uint32_t imsic_num_bits(uint32_t count)
 {
     uint32_t ret = 0;
 
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 4ef1f660ab..00c22492a7 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -23,6 +23,7 @@
 #include "hw/riscv/riscv_hart.h"
 #include "hw/sysbus.h"
 #include "hw/block/flash.h"
+#include "hw/intc/riscv_imsic.h"
 
 #define VIRT_CPUS_MAX_BITS             9
 #define VIRT_CPUS_MAX                  (1 << VIRT_CPUS_MAX_BITS)
@@ -128,4 +129,28 @@ enum {
 
 bool virt_is_acpi_enabled(RISCVVirtState *s);
 void virt_acpi_setup(RISCVVirtState *vms);
+uint32_t imsic_num_bits(uint32_t count);
+
+/*
+ * The virt machine physical address space used by some of the devices
+ * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
+ * number of CPUs, and number of IMSIC guest files.
+ *
+ * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
+ * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
+ * of virt machine physical address space.
+ */
+
+#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
+#if VIRT_IMSIC_GROUP_MAX_SIZE < \
+    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
+#error "Can't accomodate single IMSIC group in address space"
+#endif
+
+#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
+                                        VIRT_IMSIC_GROUP_MAX_SIZE)
+#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
+#error "Can't accomodate all IMSIC groups in address space"
+#endif
+
 #endif
-- 
2.39.2

Re: [PATCH 03/10] hw/riscv: virt: Make few IMSIC macros and functions public

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> Some macros and static function related to IMSIC are defined
> in virt.c. They are required in virt-acpi-build.c. So, make them
> public.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt.c         | 25 +------------------------
>   include/hw/riscv/virt.h | 25 +++++++++++++++++++++++++
>   2 files changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 46d3341113..f6067db8ec 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -37,7 +37,6 @@
>   #include "hw/riscv/numa.h"
>   #include "hw/intc/riscv_aclint.h"
>   #include "hw/intc/riscv_aplic.h"
> -#include "hw/intc/riscv_imsic.h"
>   #include "hw/intc/sifive_plic.h"
>   #include "hw/misc/sifive_test.h"
>   #include "hw/platform-bus.h"
> @@ -53,28 +52,6 @@
>   #include "hw/acpi/aml-build.h"
>   #include "qapi/qapi-visit-common.h"
>   
> -/*
> - * The virt machine physical address space used by some of the devices
> - * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
> - * number of CPUs, and number of IMSIC guest files.
> - *
> - * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
> - * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
> - * of virt machine physical address space.
> - */
> -
> -#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
> -#if VIRT_IMSIC_GROUP_MAX_SIZE < \
> -    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
> -#error "Can't accomodate single IMSIC group in address space"
> -#endif
> -
> -#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
> -                                        VIRT_IMSIC_GROUP_MAX_SIZE)
> -#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
> -#error "Can't accomodate all IMSIC groups in address space"
> -#endif
> -
>   static const MemMapEntry virt_memmap[] = {
>       [VIRT_DEBUG] =        {        0x0,         0x100 },
>       [VIRT_MROM] =         {     0x1000,        0xf000 },
> @@ -505,7 +482,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>       g_free(plic_cells);
>   }
>   
> -static uint32_t imsic_num_bits(uint32_t count)
> +uint32_t imsic_num_bits(uint32_t count)
>   {
>       uint32_t ret = 0;
>   
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 4ef1f660ab..00c22492a7 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -23,6 +23,7 @@
>   #include "hw/riscv/riscv_hart.h"
>   #include "hw/sysbus.h"
>   #include "hw/block/flash.h"
> +#include "hw/intc/riscv_imsic.h"
>   
>   #define VIRT_CPUS_MAX_BITS             9
>   #define VIRT_CPUS_MAX                  (1 << VIRT_CPUS_MAX_BITS)
> @@ -128,4 +129,28 @@ enum {
>   
>   bool virt_is_acpi_enabled(RISCVVirtState *s);
>   void virt_acpi_setup(RISCVVirtState *vms);
> +uint32_t imsic_num_bits(uint32_t count);
> +
> +/*
> + * The virt machine physical address space used by some of the devices
> + * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
> + * number of CPUs, and number of IMSIC guest files.
> + *
> + * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
> + * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
> + * of virt machine physical address space.
> + */
> +
> +#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
> +#if VIRT_IMSIC_GROUP_MAX_SIZE < \
> +    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
> +#error "Can't accomodate single IMSIC group in address space"
> +#endif
> +
> +#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
> +                                        VIRT_IMSIC_GROUP_MAX_SIZE)
> +#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
> +#error "Can't accomodate all IMSIC groups in address space"
> +#endif
> +
>   #endif

Re: [PATCH 03/10] hw/riscv: virt: Make few IMSIC macros and functions public

[Alistair Francis]

On Thu, Jul 13, 2023 at 2:42 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> Some macros and static function related to IMSIC are defined
> in virt.c. They are required in virt-acpi-build.c. So, make them
> public.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c         | 25 +------------------------
>  include/hw/riscv/virt.h | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 46d3341113..f6067db8ec 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -37,7 +37,6 @@
>  #include "hw/riscv/numa.h"
>  #include "hw/intc/riscv_aclint.h"
>  #include "hw/intc/riscv_aplic.h"
> -#include "hw/intc/riscv_imsic.h"
>  #include "hw/intc/sifive_plic.h"
>  #include "hw/misc/sifive_test.h"
>  #include "hw/platform-bus.h"
> @@ -53,28 +52,6 @@
>  #include "hw/acpi/aml-build.h"
>  #include "qapi/qapi-visit-common.h"
>
> -/*
> - * The virt machine physical address space used by some of the devices
> - * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
> - * number of CPUs, and number of IMSIC guest files.
> - *
> - * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
> - * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
> - * of virt machine physical address space.
> - */
> -
> -#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
> -#if VIRT_IMSIC_GROUP_MAX_SIZE < \
> -    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
> -#error "Can't accomodate single IMSIC group in address space"
> -#endif
> -
> -#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
> -                                        VIRT_IMSIC_GROUP_MAX_SIZE)
> -#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
> -#error "Can't accomodate all IMSIC groups in address space"
> -#endif
> -
>  static const MemMapEntry virt_memmap[] = {
>      [VIRT_DEBUG] =        {        0x0,         0x100 },
>      [VIRT_MROM] =         {     0x1000,        0xf000 },
> @@ -505,7 +482,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      g_free(plic_cells);
>  }
>
> -static uint32_t imsic_num_bits(uint32_t count)
> +uint32_t imsic_num_bits(uint32_t count)
>  {
>      uint32_t ret = 0;
>
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 4ef1f660ab..00c22492a7 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -23,6 +23,7 @@
>  #include "hw/riscv/riscv_hart.h"
>  #include "hw/sysbus.h"
>  #include "hw/block/flash.h"
> +#include "hw/intc/riscv_imsic.h"
>
>  #define VIRT_CPUS_MAX_BITS             9
>  #define VIRT_CPUS_MAX                  (1 << VIRT_CPUS_MAX_BITS)
> @@ -128,4 +129,28 @@ enum {
>
>  bool virt_is_acpi_enabled(RISCVVirtState *s);
>  void virt_acpi_setup(RISCVVirtState *vms);
> +uint32_t imsic_num_bits(uint32_t count);
> +
> +/*
> + * The virt machine physical address space used by some of the devices
> + * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets,
> + * number of CPUs, and number of IMSIC guest files.
> + *
> + * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS,
> + * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization
> + * of virt machine physical address space.
> + */
> +
> +#define VIRT_IMSIC_GROUP_MAX_SIZE      (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
> +#if VIRT_IMSIC_GROUP_MAX_SIZE < \
> +    IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
> +#error "Can't accomodate single IMSIC group in address space"
> +#endif
> +
> +#define VIRT_IMSIC_MAX_SIZE            (VIRT_SOCKETS_MAX * \
> +                                        VIRT_IMSIC_GROUP_MAX_SIZE)
> +#if 0x4000000 < VIRT_IMSIC_MAX_SIZE
> +#error "Can't accomodate all IMSIC groups in address space"
> +#endif
> +
>  #endif
> --
> 2.39.2
>
>

[PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Sunil V L]

PCIe High MMIO base is actually dynamic and fixed at
run time based on the RAM configured. Currently, this is
not part of the memmap and kept in separate static variable
in virt.c. However, ACPI code also needs this information
to populate DSDT. So, once the base is discovered, merge
this into the final memmap which can be used to create
ACPI tables later.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt.c         | 31 ++++++++++++++++++++++++++++++-
 include/hw/riscv/virt.h |  9 +++++++--
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f6067db8ec..7aee06f021 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -84,6 +84,22 @@ static const MemMapEntry virt_memmap[] = {
 
 static MemMapEntry virt_high_pcie_memmap;
 
+/*
+ * virt_memmap doesn't include floating High Mem IO address entry. To enable
+ * code organization in multiple files (ex: ACPI), it is better to have single
+ * memmap which has complete information.
+ *
+ * VIRT_HIGH_PCIE_MMIO is always greater than the last memmap entry and hence
+ * full_virt_memmap is capable of holding both virt_memmap and
+ * VIRT_HIGH_PCIE_MMIO entry.
+ *
+ * The values for these floating entries will be updated when top of RAM is
+ * discovered.
+ */
+static MemMapEntry full_virt_memmap[] = {
+    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, 0 },
+};
+
 #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
 
 static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
@@ -1444,7 +1460,20 @@ static void virt_machine_init(MachineState *machine)
             ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
     }
 
-    s->memmap = virt_memmap;
+    /*
+     * Initialize the floating values in full memory map
+     */
+    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].base = virt_high_pcie_memmap.base;
+    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].size = virt_high_pcie_memmap.size;
+
+    s->memmap = full_virt_memmap;
+    /*
+     * Copy the base virt_memmap entries to full memmap
+     */
+    for (i = 0; i < ARRAY_SIZE(virt_memmap); i++) {
+        s->memmap[i] = virt_memmap[i];
+    }
+
 
     /* register system main memory (actual RAM) */
     memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 00c22492a7..1d7ddf5df0 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -60,7 +60,7 @@ struct RISCVVirtState {
     char *oem_id;
     char *oem_table_id;
     OnOffAuto acpi;
-    const MemMapEntry *memmap;
+    MemMapEntry *memmap;
     PCIBus *bus;
 };
 
@@ -84,7 +84,12 @@ enum {
     VIRT_PCIE_MMIO,
     VIRT_PCIE_PIO,
     VIRT_PLATFORM_BUS,
-    VIRT_PCIE_ECAM
+    VIRT_PCIE_ECAM,
+    VIRT_LAST_MEMMAP /* Keep this entry always last */
+};
+
+enum {
+    VIRT_HIGH_PCIE_MMIO = VIRT_LAST_MEMMAP,
 };
 
 enum {
-- 
2.39.2

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> PCIe High MMIO base is actually dynamic and fixed at
> run time based on the RAM configured. Currently, this is
> not part of the memmap and kept in separate static variable
> in virt.c. However, ACPI code also needs this information
> to populate DSDT. So, once the base is discovered, merge
> this into the final memmap which can be used to create
> ACPI tables later.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>   hw/riscv/virt.c         | 31 ++++++++++++++++++++++++++++++-
>   include/hw/riscv/virt.h |  9 +++++++--
>   2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index f6067db8ec..7aee06f021 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -84,6 +84,22 @@ static const MemMapEntry virt_memmap[] = {
>   
>   static MemMapEntry virt_high_pcie_memmap;
>   
> +/*
> + * virt_memmap doesn't include floating High Mem IO address entry. To enable
> + * code organization in multiple files (ex: ACPI), it is better to have single
> + * memmap which has complete information.
> + *
> + * VIRT_HIGH_PCIE_MMIO is always greater than the last memmap entry and hence
> + * full_virt_memmap is capable of holding both virt_memmap and
> + * VIRT_HIGH_PCIE_MMIO entry.
> + *
> + * The values for these floating entries will be updated when top of RAM is
> + * discovered.
> + */
> +static MemMapEntry full_virt_memmap[] = {
> +    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, 0 },
> +};
> +
>   #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
>   
>   static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> @@ -1444,7 +1460,20 @@ static void virt_machine_init(MachineState *machine)
>               ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
>       }
>   
> -    s->memmap = virt_memmap;
> +    /*
> +     * Initialize the floating values in full memory map
> +     */
> +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].base = virt_high_pcie_memmap.base;
> +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].size = virt_high_pcie_memmap.size;
> +
> +    s->memmap = full_virt_memmap;
> +    /*
> +     * Copy the base virt_memmap entries to full memmap
> +     */
> +    for (i = 0; i < ARRAY_SIZE(virt_memmap); i++) {
> +        s->memmap[i] = virt_memmap[i];
> +    }
> +

This change here kind of convinces me of the point I made earlier in patch 2:
we can simplify gpex_pcie_init() to use just the RISCVVirtState as a parameter
and get everything else from it.

It's also something for a follow-up. As for this patch:

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   
>       /* register system main memory (actual RAM) */
>       memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 00c22492a7..1d7ddf5df0 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -60,7 +60,7 @@ struct RISCVVirtState {
>       char *oem_id;
>       char *oem_table_id;
>       OnOffAuto acpi;
> -    const MemMapEntry *memmap;
> +    MemMapEntry *memmap;
>       PCIBus *bus;
>   };
>   
> @@ -84,7 +84,12 @@ enum {
>       VIRT_PCIE_MMIO,
>       VIRT_PCIE_PIO,
>       VIRT_PLATFORM_BUS,
> -    VIRT_PCIE_ECAM
> +    VIRT_PCIE_ECAM,
> +    VIRT_LAST_MEMMAP /* Keep this entry always last */
> +};
> +
> +enum {
> +    VIRT_HIGH_PCIE_MMIO = VIRT_LAST_MEMMAP,
>   };
>   
>   enum {

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Sunil V L]

On Tue, Jul 18, 2023 at 05:05:12PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 7/12/23 13:39, Sunil V L wrote:
> > PCIe High MMIO base is actually dynamic and fixed at
> > run time based on the RAM configured. Currently, this is
> > not part of the memmap and kept in separate static variable
> > in virt.c. However, ACPI code also needs this information
> > to populate DSDT. So, once the base is discovered, merge
> > this into the final memmap which can be used to create
> > ACPI tables later.
> > 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
> >   hw/riscv/virt.c         | 31 ++++++++++++++++++++++++++++++-
> >   include/hw/riscv/virt.h |  9 +++++++--
> >   2 files changed, 37 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index f6067db8ec..7aee06f021 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -84,6 +84,22 @@ static const MemMapEntry virt_memmap[] = {
> >   static MemMapEntry virt_high_pcie_memmap;
> > +/*
> > + * virt_memmap doesn't include floating High Mem IO address entry. To enable
> > + * code organization in multiple files (ex: ACPI), it is better to have single
> > + * memmap which has complete information.
> > + *
> > + * VIRT_HIGH_PCIE_MMIO is always greater than the last memmap entry and hence
> > + * full_virt_memmap is capable of holding both virt_memmap and
> > + * VIRT_HIGH_PCIE_MMIO entry.
> > + *
> > + * The values for these floating entries will be updated when top of RAM is
> > + * discovered.
> > + */
> > +static MemMapEntry full_virt_memmap[] = {
> > +    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, 0 },
> > +};
> > +
> >   #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
> >   static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> > @@ -1444,7 +1460,20 @@ static void virt_machine_init(MachineState *machine)
> >               ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
> >       }
> > -    s->memmap = virt_memmap;
> > +    /*
> > +     * Initialize the floating values in full memory map
> > +     */
> > +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].base = virt_high_pcie_memmap.base;
> > +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].size = virt_high_pcie_memmap.size;
> > +
> > +    s->memmap = full_virt_memmap;
> > +    /*
> > +     * Copy the base virt_memmap entries to full memmap
> > +     */
> > +    for (i = 0; i < ARRAY_SIZE(virt_memmap); i++) {
> > +        s->memmap[i] = virt_memmap[i];
> > +    }
> > +
> 
> This change here kind of convinces me of the point I made earlier in patch 2:
> we can simplify gpex_pcie_init() to use just the RISCVVirtState as a parameter
> and get everything else from it.
> 
> It's also something for a follow-up. As for this patch:
> 
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> 
Thanks Daniel. I agree. I can send another follow-up patch to simplify
gpex_pcie_init.

Thanks,
Sunil

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Igor Mammedov]

On Wed, 12 Jul 2023 22:09:37 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> PCIe High MMIO base is actually dynamic and fixed at
> run time based on the RAM configured. Currently, this is
> not part of the memmap and kept in separate static variable
> in virt.c. However, ACPI code also needs this information
> to populate DSDT. So, once the base is discovered, merge
> this into the final memmap which can be used to create
> ACPI tables later.

can ACPI code fetch virt_high_pcie_memmap at runtime from
host bridge (like we do in pc/q35)?


> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  hw/riscv/virt.c         | 31 ++++++++++++++++++++++++++++++-
>  include/hw/riscv/virt.h |  9 +++++++--
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index f6067db8ec..7aee06f021 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -84,6 +84,22 @@ static const MemMapEntry virt_memmap[] = {
>  
>  static MemMapEntry virt_high_pcie_memmap;
>  
> +/*
> + * virt_memmap doesn't include floating High Mem IO address entry. To enable
> + * code organization in multiple files (ex: ACPI), it is better to have single
> + * memmap which has complete information.
> + *
> + * VIRT_HIGH_PCIE_MMIO is always greater than the last memmap entry and hence
> + * full_virt_memmap is capable of holding both virt_memmap and
> + * VIRT_HIGH_PCIE_MMIO entry.
> + *
> + * The values for these floating entries will be updated when top of RAM is
> + * discovered.
> + */
> +static MemMapEntry full_virt_memmap[] = {
> +    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, 0 },
> +};
> +
>  #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
>  
>  static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> @@ -1444,7 +1460,20 @@ static void virt_machine_init(MachineState *machine)
>              ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
>      }
>  
> -    s->memmap = virt_memmap;
> +    /*
> +     * Initialize the floating values in full memory map
> +     */
> +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].base = virt_high_pcie_memmap.base;
> +    full_virt_memmap[VIRT_HIGH_PCIE_MMIO].size = virt_high_pcie_memmap.size;
> +
> +    s->memmap = full_virt_memmap;
> +    /*
> +     * Copy the base virt_memmap entries to full memmap
> +     */
> +    for (i = 0; i < ARRAY_SIZE(virt_memmap); i++) {
> +        s->memmap[i] = virt_memmap[i];
> +    }
> +
>  
>      /* register system main memory (actual RAM) */
>      memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 00c22492a7..1d7ddf5df0 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -60,7 +60,7 @@ struct RISCVVirtState {
>      char *oem_id;
>      char *oem_table_id;
>      OnOffAuto acpi;
> -    const MemMapEntry *memmap;
> +    MemMapEntry *memmap;
>      PCIBus *bus;
>  };
>  
> @@ -84,7 +84,12 @@ enum {
>      VIRT_PCIE_MMIO,
>      VIRT_PCIE_PIO,
>      VIRT_PLATFORM_BUS,
> -    VIRT_PCIE_ECAM
> +    VIRT_PCIE_ECAM,
> +    VIRT_LAST_MEMMAP /* Keep this entry always last */
> +};
> +
> +enum {
> +    VIRT_HIGH_PCIE_MMIO = VIRT_LAST_MEMMAP,
>  };
>  
>  enum {

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Sunil V L]

On Mon, Jul 24, 2023 at 05:32:54PM +0200, Igor Mammedov wrote:
> On Wed, 12 Jul 2023 22:09:37 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > PCIe High MMIO base is actually dynamic and fixed at
> > run time based on the RAM configured. Currently, this is
> > not part of the memmap and kept in separate static variable
> > in virt.c. However, ACPI code also needs this information
> > to populate DSDT. So, once the base is discovered, merge
> > this into the final memmap which can be used to create
> > ACPI tables later.
> 
> can ACPI code fetch virt_high_pcie_memmap at runtime from
> host bridge (like we do in pc/q35)?
> 
Hi Igor,

Looking at the current design of virt machines (arm/loongarch/riscv),
getting this directly from the memmap looks simpler. ARM ACPI also uses
the memmap to get the pcie_high space. It appears to me we need some
more infrastructure code if ACPI needs to fetch from host bridge. I am
not sure why that would be beneficial.

Thanks,
Sunil

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Igor Mammedov]

On Thu, 27 Jul 2023 16:29:19 +0530
Sunil V L <sunilvl@ventanamicro.com> wrote:

> On Mon, Jul 24, 2023 at 05:32:54PM +0200, Igor Mammedov wrote:
> > On Wed, 12 Jul 2023 22:09:37 +0530
> > Sunil V L <sunilvl@ventanamicro.com> wrote:
> >   
> > > PCIe High MMIO base is actually dynamic and fixed at
> > > run time based on the RAM configured. Currently, this is
> > > not part of the memmap and kept in separate static variable
> > > in virt.c. However, ACPI code also needs this information
> > > to populate DSDT. So, once the base is discovered, merge
> > > this into the final memmap which can be used to create
> > > ACPI tables later.  
> > 
> > can ACPI code fetch virt_high_pcie_memmap at runtime from
> > host bridge (like we do in pc/q35)?
> >   
> Hi Igor,
> 
> Looking at the current design of virt machines (arm/loongarch/riscv),
> getting this directly from the memmap looks simpler. ARM ACPI also uses
> the memmap to get the pcie_high space. It appears to me we need some
> more infrastructure code if ACPI needs to fetch from host bridge. I am
> not sure why that would be beneficial.

Sure it's possible to directly hardcode access, but it becomes machine
specific and hard to generalize/reuse (defaults might belong to machine,
but ACPI shall pickup these from actual owner - hostbridge).

And no extra infrastructure is need, x86 manages to do that by
using properties on host bridge (one can pre-program values on host bridge
during it's creation, firmware can also change them later when initializing PCI).
Then DSDT generator picks up uptodate values from hostbridge
(which is actual owner of these values) via properties.
(basically copy pc/q35 approach). 

> 
> Thanks,
> Sunil
> 

Re: [PATCH 04/10] hw/riscv: virt: Add PCIe HIGHMEM in memmap

[Sunil V L]

On Thu, Jul 27, 2023 at 02:04:46PM +0200, Igor Mammedov wrote:
> On Thu, 27 Jul 2023 16:29:19 +0530
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > On Mon, Jul 24, 2023 at 05:32:54PM +0200, Igor Mammedov wrote:
> > > On Wed, 12 Jul 2023 22:09:37 +0530
> > > Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >   
> > > > PCIe High MMIO base is actually dynamic and fixed at
> > > > run time based on the RAM configured. Currently, this is
> > > > not part of the memmap and kept in separate static variable
> > > > in virt.c. However, ACPI code also needs this information
> > > > to populate DSDT. So, once the base is discovered, merge
> > > > this into the final memmap which can be used to create
> > > > ACPI tables later.  
> > > 
> > > can ACPI code fetch virt_high_pcie_memmap at runtime from
> > > host bridge (like we do in pc/q35)?
> > >   
> > Hi Igor,
> > 
> > Looking at the current design of virt machines (arm/loongarch/riscv),
> > getting this directly from the memmap looks simpler. ARM ACPI also uses
> > the memmap to get the pcie_high space. It appears to me we need some
> > more infrastructure code if ACPI needs to fetch from host bridge. I am
> > not sure why that would be beneficial.
> 
> Sure it's possible to directly hardcode access, but it becomes machine
> specific and hard to generalize/reuse (defaults might belong to machine,
> but ACPI shall pickup these from actual owner - hostbridge).
> 
> And no extra infrastructure is need, x86 manages to do that by
> using properties on host bridge (one can pre-program values on host bridge
> during it's creation, firmware can also change them later when initializing PCI).
> Then DSDT generator picks up uptodate values from hostbridge
> (which is actual owner of these values) via properties.
> (basically copy pc/q35 approach). 
> 
Ahh OK. Thanks!. Let me update.

Thanks,
Sunil

[PATCH 05/10] hw/riscv/virt-acpi-build.c: Add AIA support in RINTC

[Sunil V L]

Update the RINTC structure in MADT with AIA related fields.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 66 +++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 01843e4509..12b8ef0352 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -37,6 +37,7 @@
 #include "hw/intc/riscv_aclint.h"
 
 #define ACPI_BUILD_TABLE_SIZE             0x20000
+#define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
 
 typedef struct AcpiBuildState {
     /* Copy of table in RAM (for patching) */
@@ -57,18 +58,42 @@ static void acpi_align_size(GArray *blob, unsigned align)
 }
 
 static void riscv_acpi_madt_add_rintc(uint32_t uid,
+                                      uint32_t local_cpu_id,
                                       const CPUArchIdList *arch_ids,
-                                      GArray *entry)
+                                      GArray *entry,
+                                      RISCVVirtAIAType aia_type,
+                                      uint64_t imsic_addr,
+                                      uint32_t imsic_size)
 {
     uint64_t hart_id = arch_ids->cpus[uid].arch_id;
 
     build_append_int_noprefix(entry, 0x18, 1);       /* Type     */
-    build_append_int_noprefix(entry, 20, 1);         /* Length   */
+    build_append_int_noprefix(entry, 36, 1);         /* Length   */
     build_append_int_noprefix(entry, 1, 1);          /* Version  */
     build_append_int_noprefix(entry, 0, 1);          /* Reserved */
     build_append_int_noprefix(entry, 0x1, 4);        /* Flags    */
     build_append_int_noprefix(entry, hart_id, 8);    /* Hart ID  */
     build_append_int_noprefix(entry, uid, 4);        /* ACPI Processor UID */
+    /* External Interrupt Controller ID */
+    if (aia_type == VIRT_AIA_TYPE_APLIC) {
+        build_append_int_noprefix(entry,
+                                  ACPI_BUILD_INTC_ID(
+                                      arch_ids->cpus[uid].props.node_id,
+                                      local_cpu_id),
+                                  4);
+    } else {
+        build_append_int_noprefix(entry, 0, 4);
+    }
+
+    if (aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
+        /* IMSIC Base address */
+        build_append_int_noprefix(entry, imsic_addr, 8);
+        /* IMSIC Size */
+        build_append_int_noprefix(entry, imsic_size, 4);
+    } else {
+        build_append_int_noprefix(entry, 0, 8);
+        build_append_int_noprefix(entry, 0, 4);
+    }
 }
 
 static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
@@ -76,6 +101,11 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
     MachineClass *mc = MACHINE_GET_CLASS(s);
     MachineState *ms = MACHINE(s);
     const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
+    uint64_t imsic_socket_addr, imsic_addr;
+    uint8_t  guest_index_bits;
+    uint32_t imsic_size, local_cpu_id, socket_id;
+
+    guest_index_bits = imsic_num_bits(s->aia_guests + 1);
 
     for (int i = 0; i < arch_ids->len; i++) {
             Aml *dev;
@@ -86,8 +116,19 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
             aml_append(dev, aml_name_decl("_UID",
                        aml_int(arch_ids->cpus[i].arch_id)));
 
+            socket_id = arch_ids->cpus[i].props.node_id;
+            local_cpu_id = (arch_ids->cpus[i].arch_id -
+                            riscv_socket_first_hartid(ms, socket_id)) %
+                            riscv_socket_hart_count(ms, socket_id);
             /* build _MAT object */
-            riscv_acpi_madt_add_rintc(i, arch_ids, madt_buf);
+            imsic_socket_addr = s->memmap[VIRT_IMSIC_S].base +
+                                (socket_id * VIRT_IMSIC_GROUP_MAX_SIZE);
+            imsic_addr = imsic_socket_addr +
+                         local_cpu_id * IMSIC_HART_SIZE(guest_index_bits);
+            imsic_size = IMSIC_HART_SIZE(guest_index_bits);
+
+            riscv_acpi_madt_add_rintc(i, local_cpu_id, arch_ids, madt_buf,
+                                      s->aia_type, imsic_addr, imsic_size);
             aml_append(dev, aml_name_decl("_MAT",
                                           aml_buffer(madt_buf->len,
                                           (uint8_t *)madt_buf->data)));
@@ -226,6 +267,7 @@ static void build_dsdt(GArray *table_data,
  * 5.2.12 Multiple APIC Description Table (MADT)
  * REF: https://github.com/riscv-non-isa/riscv-acpi/issues/15
  *      https://drive.google.com/file/d/1R6k4MshhN3WTT-hwqAquu5nX6xSEqK2l/view
+ *      https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view
  */
 static void build_madt(GArray *table_data,
                        BIOSLinker *linker,
@@ -234,6 +276,12 @@ static void build_madt(GArray *table_data,
     MachineClass *mc = MACHINE_GET_CLASS(s);
     MachineState *ms = MACHINE(s);
     const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
+    uint64_t imsic_socket_addr, imsic_addr;
+    uint8_t  guest_index_bits;
+    uint32_t imsic_size;
+    uint32_t local_cpu_id, socket_id;
+
+    guest_index_bits = imsic_num_bits(s->aia_guests + 1);
 
     AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id,
                         .oem_table_id = s->oem_table_id };
@@ -245,7 +293,17 @@ static void build_madt(GArray *table_data,
 
     /* RISC-V Local INTC structures per HART */
     for (int i = 0; i < arch_ids->len; i++) {
-        riscv_acpi_madt_add_rintc(i, arch_ids, table_data);
+        socket_id = arch_ids->cpus[i].props.node_id;
+        local_cpu_id = (arch_ids->cpus[i].arch_id -
+                       riscv_socket_first_hartid(ms, socket_id)) %
+                       riscv_socket_hart_count(ms, socket_id);
+        imsic_socket_addr = s->memmap[VIRT_IMSIC_S].base +
+                            (socket_id * VIRT_IMSIC_GROUP_MAX_SIZE);
+        imsic_addr = imsic_socket_addr +
+                     local_cpu_id * IMSIC_HART_SIZE(guest_index_bits);
+        imsic_size = IMSIC_HART_SIZE(guest_index_bits);
+        riscv_acpi_madt_add_rintc(i, local_cpu_id, arch_ids, table_data,
+                                  s->aia_type, imsic_addr, imsic_size);
     }
 
     acpi_table_end(linker, &table);
-- 
2.39.2

Re: [PATCH 05/10] hw/riscv/virt-acpi-build.c: Add AIA support in RINTC

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> Update the RINTC structure in MADT with AIA related fields.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt-acpi-build.c | 66 +++++++++++++++++++++++++++++++++++---
>   1 file changed, 62 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 01843e4509..12b8ef0352 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -37,6 +37,7 @@
>   #include "hw/intc/riscv_aclint.h"
>   
>   #define ACPI_BUILD_TABLE_SIZE             0x20000
> +#define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
>   
>   typedef struct AcpiBuildState {
>       /* Copy of table in RAM (for patching) */
> @@ -57,18 +58,42 @@ static void acpi_align_size(GArray *blob, unsigned align)
>   }
>   
>   static void riscv_acpi_madt_add_rintc(uint32_t uid,
> +                                      uint32_t local_cpu_id,
>                                         const CPUArchIdList *arch_ids,
> -                                      GArray *entry)
> +                                      GArray *entry,
> +                                      RISCVVirtAIAType aia_type,
> +                                      uint64_t imsic_addr,
> +                                      uint32_t imsic_size)
>   {
>       uint64_t hart_id = arch_ids->cpus[uid].arch_id;
>   
>       build_append_int_noprefix(entry, 0x18, 1);       /* Type     */
> -    build_append_int_noprefix(entry, 20, 1);         /* Length   */
> +    build_append_int_noprefix(entry, 36, 1);         /* Length   */
>       build_append_int_noprefix(entry, 1, 1);          /* Version  */
>       build_append_int_noprefix(entry, 0, 1);          /* Reserved */
>       build_append_int_noprefix(entry, 0x1, 4);        /* Flags    */
>       build_append_int_noprefix(entry, hart_id, 8);    /* Hart ID  */
>       build_append_int_noprefix(entry, uid, 4);        /* ACPI Processor UID */
> +    /* External Interrupt Controller ID */
> +    if (aia_type == VIRT_AIA_TYPE_APLIC) {
> +        build_append_int_noprefix(entry,
> +                                  ACPI_BUILD_INTC_ID(
> +                                      arch_ids->cpus[uid].props.node_id,
> +                                      local_cpu_id),
> +                                  4);
> +    } else {
> +        build_append_int_noprefix(entry, 0, 4);
> +    }
> +
> +    if (aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
> +        /* IMSIC Base address */
> +        build_append_int_noprefix(entry, imsic_addr, 8);
> +        /* IMSIC Size */
> +        build_append_int_noprefix(entry, imsic_size, 4);
> +    } else {
> +        build_append_int_noprefix(entry, 0, 8);
> +        build_append_int_noprefix(entry, 0, 4);
> +    }
>   }
>   
>   static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
> @@ -76,6 +101,11 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>       MachineClass *mc = MACHINE_GET_CLASS(s);
>       MachineState *ms = MACHINE(s);
>       const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
> +    uint64_t imsic_socket_addr, imsic_addr;
> +    uint8_t  guest_index_bits;
> +    uint32_t imsic_size, local_cpu_id, socket_id;
> +
> +    guest_index_bits = imsic_num_bits(s->aia_guests + 1);
>   
>       for (int i = 0; i < arch_ids->len; i++) {
>               Aml *dev;
> @@ -86,8 +116,19 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>               aml_append(dev, aml_name_decl("_UID",
>                          aml_int(arch_ids->cpus[i].arch_id)));
>   
> +            socket_id = arch_ids->cpus[i].props.node_id;
> +            local_cpu_id = (arch_ids->cpus[i].arch_id -
> +                            riscv_socket_first_hartid(ms, socket_id)) %
> +                            riscv_socket_hart_count(ms, socket_id);
>               /* build _MAT object */
> -            riscv_acpi_madt_add_rintc(i, arch_ids, madt_buf);
> +            imsic_socket_addr = s->memmap[VIRT_IMSIC_S].base +
> +                                (socket_id * VIRT_IMSIC_GROUP_MAX_SIZE);
> +            imsic_addr = imsic_socket_addr +
> +                         local_cpu_id * IMSIC_HART_SIZE(guest_index_bits);
> +            imsic_size = IMSIC_HART_SIZE(guest_index_bits);
> +
> +            riscv_acpi_madt_add_rintc(i, local_cpu_id, arch_ids, madt_buf,
> +                                      s->aia_type, imsic_addr, imsic_size);
>               aml_append(dev, aml_name_decl("_MAT",
>                                             aml_buffer(madt_buf->len,
>                                             (uint8_t *)madt_buf->data)));
> @@ -226,6 +267,7 @@ static void build_dsdt(GArray *table_data,
>    * 5.2.12 Multiple APIC Description Table (MADT)
>    * REF: https://github.com/riscv-non-isa/riscv-acpi/issues/15
>    *      https://drive.google.com/file/d/1R6k4MshhN3WTT-hwqAquu5nX6xSEqK2l/view
> + *      https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view
>    */
>   static void build_madt(GArray *table_data,
>                          BIOSLinker *linker,
> @@ -234,6 +276,12 @@ static void build_madt(GArray *table_data,
>       MachineClass *mc = MACHINE_GET_CLASS(s);
>       MachineState *ms = MACHINE(s);
>       const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
> +    uint64_t imsic_socket_addr, imsic_addr;
> +    uint8_t  guest_index_bits;
> +    uint32_t imsic_size;
> +    uint32_t local_cpu_id, socket_id;
> +
> +    guest_index_bits = imsic_num_bits(s->aia_guests + 1);
>   
>       AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id,
>                           .oem_table_id = s->oem_table_id };
> @@ -245,7 +293,17 @@ static void build_madt(GArray *table_data,
>   
>       /* RISC-V Local INTC structures per HART */
>       for (int i = 0; i < arch_ids->len; i++) {
> -        riscv_acpi_madt_add_rintc(i, arch_ids, table_data);
> +        socket_id = arch_ids->cpus[i].props.node_id;
> +        local_cpu_id = (arch_ids->cpus[i].arch_id -
> +                       riscv_socket_first_hartid(ms, socket_id)) %
> +                       riscv_socket_hart_count(ms, socket_id);
> +        imsic_socket_addr = s->memmap[VIRT_IMSIC_S].base +
> +                            (socket_id * VIRT_IMSIC_GROUP_MAX_SIZE);
> +        imsic_addr = imsic_socket_addr +
> +                     local_cpu_id * IMSIC_HART_SIZE(guest_index_bits);
> +        imsic_size = IMSIC_HART_SIZE(guest_index_bits);
> +        riscv_acpi_madt_add_rintc(i, local_cpu_id, arch_ids, table_data,
> +                                  s->aia_type, imsic_addr, imsic_size);
>       }
>   
>       acpi_table_end(linker, &table);

[PATCH 06/10] hw/riscv/virt-acpi-build.c: Add IMSIC in the MADT

[Sunil V L]

Add IMSIC structure in MADT when IMSIC is configured.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 12b8ef0352..ebdc3bffea 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -280,8 +280,20 @@ static void build_madt(GArray *table_data,
     uint8_t  guest_index_bits;
     uint32_t imsic_size;
     uint32_t local_cpu_id, socket_id;
+    uint8_t  hart_index_bits, group_index_bits, group_index_shift;
+    uint16_t imsic_max_hart_per_socket = 0;
+    uint8_t  socket;
+
+    for (socket = 0; socket < riscv_socket_count(ms); socket++) {
+        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
+            imsic_max_hart_per_socket = s->soc[socket].num_harts;
+        }
+    }
 
     guest_index_bits = imsic_num_bits(s->aia_guests + 1);
+    hart_index_bits = imsic_num_bits(imsic_max_hart_per_socket);
+    group_index_bits = imsic_num_bits(riscv_socket_count(ms));
+    group_index_shift = IMSIC_MMIO_GROUP_MIN_SHIFT;
 
     AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id,
                         .oem_table_id = s->oem_table_id };
@@ -306,6 +318,28 @@ static void build_madt(GArray *table_data,
                                   s->aia_type, imsic_addr, imsic_size);
     }
 
+    /* IMSIC */
+    if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
+        /* IMSIC */
+        build_append_int_noprefix(table_data, 0x19, 1);     /* Type */
+        build_append_int_noprefix(table_data, 16, 1);       /* Length */
+        build_append_int_noprefix(table_data, 1, 1);        /* Version */
+        build_append_int_noprefix(table_data, 0, 1);        /* Reserved */
+        build_append_int_noprefix(table_data, 0, 4);        /* Flags */
+        /* Number of supervisor mode Interrupt Identities */
+        build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_MSIS, 2);
+        /* Number of guest mode Interrupt Identities */
+        build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_MSIS, 2);
+        /* Guest Index Bits */
+        build_append_int_noprefix(table_data, guest_index_bits, 1);
+        /* Hart Index Bits */
+        build_append_int_noprefix(table_data, hart_index_bits, 1);
+        /* Group Index Bits */
+        build_append_int_noprefix(table_data, group_index_bits, 1);
+        /* Group Index Shift */
+        build_append_int_noprefix(table_data, group_index_shift, 1);
+    }
+
     acpi_table_end(linker, &table);
 }
 
-- 
2.39.2

Re: [PATCH 06/10] hw/riscv/virt-acpi-build.c: Add IMSIC in the MADT

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> Add IMSIC structure in MADT when IMSIC is configured.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 12b8ef0352..ebdc3bffea 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -280,8 +280,20 @@ static void build_madt(GArray *table_data,
>       uint8_t  guest_index_bits;
>       uint32_t imsic_size;
>       uint32_t local_cpu_id, socket_id;
> +    uint8_t  hart_index_bits, group_index_bits, group_index_shift;
> +    uint16_t imsic_max_hart_per_socket = 0;
> +    uint8_t  socket;
> +
> +    for (socket = 0; socket < riscv_socket_count(ms); socket++) {
> +        if (imsic_max_hart_per_socket < s->soc[socket].num_harts) {
> +            imsic_max_hart_per_socket = s->soc[socket].num_harts;
> +        }
> +    }
>   
>       guest_index_bits = imsic_num_bits(s->aia_guests + 1);
> +    hart_index_bits = imsic_num_bits(imsic_max_hart_per_socket);
> +    group_index_bits = imsic_num_bits(riscv_socket_count(ms));
> +    group_index_shift = IMSIC_MMIO_GROUP_MIN_SHIFT;
>   
>       AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id,
>                           .oem_table_id = s->oem_table_id };
> @@ -306,6 +318,28 @@ static void build_madt(GArray *table_data,
>                                     s->aia_type, imsic_addr, imsic_size);
>       }
>   
> +    /* IMSIC */
> +    if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
> +        /* IMSIC */
> +        build_append_int_noprefix(table_data, 0x19, 1);     /* Type */
> +        build_append_int_noprefix(table_data, 16, 1);       /* Length */
> +        build_append_int_noprefix(table_data, 1, 1);        /* Version */
> +        build_append_int_noprefix(table_data, 0, 1);        /* Reserved */
> +        build_append_int_noprefix(table_data, 0, 4);        /* Flags */
> +        /* Number of supervisor mode Interrupt Identities */
> +        build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_MSIS, 2);
> +        /* Number of guest mode Interrupt Identities */
> +        build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_MSIS, 2);
> +        /* Guest Index Bits */
> +        build_append_int_noprefix(table_data, guest_index_bits, 1);
> +        /* Hart Index Bits */
> +        build_append_int_noprefix(table_data, hart_index_bits, 1);
> +        /* Group Index Bits */
> +        build_append_int_noprefix(table_data, group_index_bits, 1);
> +        /* Group Index Shift */
> +        build_append_int_noprefix(table_data, group_index_shift, 1);
> +    }
> +
>       acpi_table_end(linker, &table);
>   }
>   

[PATCH 07/10] hw/riscv/virt-acpi-build.c: Add APLIC in the MADT

[Sunil V L]

Add APLIC structures for each socket in the MADT when
system is configured with APLIC as the external wired
interrupt controller.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index ebdc3bffea..9f2d0c92b0 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -276,9 +276,9 @@ static void build_madt(GArray *table_data,
     MachineClass *mc = MACHINE_GET_CLASS(s);
     MachineState *ms = MACHINE(s);
     const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
-    uint64_t imsic_socket_addr, imsic_addr;
+    uint64_t imsic_socket_addr, imsic_addr, aplic_addr;
+    uint32_t imsic_size, gsi_base;
     uint8_t  guest_index_bits;
-    uint32_t imsic_size;
     uint32_t local_cpu_id, socket_id;
     uint8_t  hart_index_bits, group_index_bits, group_index_shift;
     uint16_t imsic_max_hart_per_socket = 0;
@@ -340,6 +340,38 @@ static void build_madt(GArray *table_data,
         build_append_int_noprefix(table_data, group_index_shift, 1);
     }
 
+    if (s->aia_type != VIRT_AIA_TYPE_NONE) {
+        /* APLICs */
+        for (socket = 0; socket < riscv_socket_count(ms); socket++) {
+            aplic_addr = s->memmap[VIRT_APLIC_S].base +
+                             s->memmap[VIRT_APLIC_S].size * socket;
+            gsi_base = VIRT_IRQCHIP_NUM_SOURCES * socket;
+            build_append_int_noprefix(table_data, 0x1A, 1);    /* Type */
+            build_append_int_noprefix(table_data, 36, 1);      /* Length */
+            build_append_int_noprefix(table_data, 1, 1);       /* Version */
+            build_append_int_noprefix(table_data, socket, 1);  /* APLIC ID */
+            build_append_int_noprefix(table_data, 0, 4);       /* Flags */
+            build_append_int_noprefix(table_data, 0, 8);       /* Hardware ID */
+            /* Number of IDCs */
+            if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
+                build_append_int_noprefix(table_data,
+                                          s->soc[socket].num_harts,
+                                          2);
+            } else {
+                build_append_int_noprefix(table_data, 0, 2);
+            }
+            /* Total External Interrupt Sources Supported */
+            build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_SOURCES, 2);
+            /* Global System Interrupt Base */
+            build_append_int_noprefix(table_data, gsi_base, 4);
+            /* APLIC Address */
+            build_append_int_noprefix(table_data, aplic_addr, 8);
+            /* APLIC size */
+            build_append_int_noprefix(table_data,
+                                      s->memmap[VIRT_APLIC_S].size, 4);
+        }
+    }
+
     acpi_table_end(linker, &table);
 }
 
-- 
2.39.2

Re: [PATCH 07/10] hw/riscv/virt-acpi-build.c: Add APLIC in the MADT

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> Add APLIC structures for each socket in the MADT when
> system is configured with APLIC as the external wired
> interrupt controller.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt-acpi-build.c | 36 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index ebdc3bffea..9f2d0c92b0 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -276,9 +276,9 @@ static void build_madt(GArray *table_data,
>       MachineClass *mc = MACHINE_GET_CLASS(s);
>       MachineState *ms = MACHINE(s);
>       const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
> -    uint64_t imsic_socket_addr, imsic_addr;
> +    uint64_t imsic_socket_addr, imsic_addr, aplic_addr;
> +    uint32_t imsic_size, gsi_base;
>       uint8_t  guest_index_bits;
> -    uint32_t imsic_size;
>       uint32_t local_cpu_id, socket_id;
>       uint8_t  hart_index_bits, group_index_bits, group_index_shift;
>       uint16_t imsic_max_hart_per_socket = 0;
> @@ -340,6 +340,38 @@ static void build_madt(GArray *table_data,
>           build_append_int_noprefix(table_data, group_index_shift, 1);
>       }
>   
> +    if (s->aia_type != VIRT_AIA_TYPE_NONE) {
> +        /* APLICs */
> +        for (socket = 0; socket < riscv_socket_count(ms); socket++) {
> +            aplic_addr = s->memmap[VIRT_APLIC_S].base +
> +                             s->memmap[VIRT_APLIC_S].size * socket;
> +            gsi_base = VIRT_IRQCHIP_NUM_SOURCES * socket;
> +            build_append_int_noprefix(table_data, 0x1A, 1);    /* Type */
> +            build_append_int_noprefix(table_data, 36, 1);      /* Length */
> +            build_append_int_noprefix(table_data, 1, 1);       /* Version */
> +            build_append_int_noprefix(table_data, socket, 1);  /* APLIC ID */
> +            build_append_int_noprefix(table_data, 0, 4);       /* Flags */
> +            build_append_int_noprefix(table_data, 0, 8);       /* Hardware ID */
> +            /* Number of IDCs */
> +            if (s->aia_type == VIRT_AIA_TYPE_APLIC) {
> +                build_append_int_noprefix(table_data,
> +                                          s->soc[socket].num_harts,
> +                                          2);
> +            } else {
> +                build_append_int_noprefix(table_data, 0, 2);
> +            }
> +            /* Total External Interrupt Sources Supported */
> +            build_append_int_noprefix(table_data, VIRT_IRQCHIP_NUM_SOURCES, 2);
> +            /* Global System Interrupt Base */
> +            build_append_int_noprefix(table_data, gsi_base, 4);
> +            /* APLIC Address */
> +            build_append_int_noprefix(table_data, aplic_addr, 8);
> +            /* APLIC size */
> +            build_append_int_noprefix(table_data,
> +                                      s->memmap[VIRT_APLIC_S].size, 4);
> +        }
> +    }
> +
>       acpi_table_end(linker, &table);
>   }
>   

[PATCH 08/10] hw/riscv/virt-acpi-build.c: Add CMO information in RHCT

[Sunil V L]

When CMO related extensions like Zicboz, Zicbom and Zicbop
are enabled, the block size for those extensions need to be
communicated via CMO node in RHCT. Add CMO node in RHCT if
any of those CMO extensions are detected.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 64 +++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 8 deletions(-)

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 9f2d0c92b0..2d2bd3b970 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -146,6 +146,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
  * 5.2.36 RISC-V Hart Capabilities Table (RHCT)
  * REF: https://github.com/riscv-non-isa/riscv-acpi/issues/16
  *      https://drive.google.com/file/d/1nP3nFiH4jkPMp6COOxP6123DCZKR-tia/view
+ *      https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view
  */
 static void build_rhct(GArray *table_data,
                        BIOSLinker *linker,
@@ -155,8 +156,8 @@ static void build_rhct(GArray *table_data,
     MachineState *ms = MACHINE(s);
     const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
     size_t len, aligned_len;
-    uint32_t isa_offset, num_rhct_nodes;
-    RISCVCPU *cpu;
+    uint32_t isa_offset, num_rhct_nodes, cmo_offset = 0;
+    RISCVCPU *cpu = &s->soc[0].harts[0];
     char *isa;
 
     AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
@@ -172,6 +173,9 @@ static void build_rhct(GArray *table_data,
 
     /* ISA + N hart info */
     num_rhct_nodes = 1 + ms->smp.cpus;
+    if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
+        num_rhct_nodes++;
+    }
 
     /* Number of RHCT nodes*/
     build_append_int_noprefix(table_data, num_rhct_nodes, 4);
@@ -183,7 +187,6 @@ static void build_rhct(GArray *table_data,
     isa_offset = table_data->len - table.table_offset;
     build_append_int_noprefix(table_data, 0, 2);   /* Type 0 */
 
-    cpu = &s->soc[0].harts[0];
     isa = riscv_isa_string(cpu);
     len = 8 + strlen(isa) + 1;
     aligned_len = (len % 2) ? (len + 1) : len;
@@ -199,14 +202,59 @@ static void build_rhct(GArray *table_data,
         build_append_int_noprefix(table_data, 0x0, 1);   /* Optional Padding */
     }
 
+    /* CMO node */
+    if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
+        cmo_offset = table_data->len - table.table_offset;
+        build_append_int_noprefix(table_data, 1, 2);    /* Type */
+        build_append_int_noprefix(table_data, 10, 2);   /* Total Length */
+        build_append_int_noprefix(table_data, 0x1, 2);  /* Revision */
+        build_append_int_noprefix(table_data, 0, 1);    /* Reserved */
+
+        /* CBOM block size */
+        if (cpu->cfg.cbom_blocksize) {
+            build_append_int_noprefix(table_data,
+                                      __builtin_ctz(cpu->cfg.cbom_blocksize),
+                                      1);
+        } else {
+            build_append_int_noprefix(table_data, 0, 1);
+        }
+
+        /* CBOP block size */
+        build_append_int_noprefix(table_data, 0, 1);
+
+        /* CBOZ block size */
+        if (cpu->cfg.cboz_blocksize) {
+            build_append_int_noprefix(table_data,
+                                      __builtin_ctz(cpu->cfg.cboz_blocksize),
+                                      1);
+        } else {
+            build_append_int_noprefix(table_data, 0, 1);
+        }
+    }
+
     /* Hart Info Node */
     for (int i = 0; i < arch_ids->len; i++) {
+        len = 16;
+        int num_offsets = 1;
         build_append_int_noprefix(table_data, 0xFFFF, 2);  /* Type */
-        build_append_int_noprefix(table_data, 16, 2);      /* Length */
-        build_append_int_noprefix(table_data, 0x1, 2);     /* Revision */
-        build_append_int_noprefix(table_data, 1, 2);    /* Number of offsets */
-        build_append_int_noprefix(table_data, i, 4);    /* ACPI Processor UID */
-        build_append_int_noprefix(table_data, isa_offset, 4); /* Offsets[0] */
+
+        /* Length */
+        if (cmo_offset) {
+            len += 4;
+            num_offsets++;
+        }
+
+        build_append_int_noprefix(table_data, len, 2);
+        build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
+        /* Number of offsets */
+        build_append_int_noprefix(table_data, num_offsets, 2);
+        build_append_int_noprefix(table_data, i, 4);   /* ACPI Processor UID */
+
+        /* Offsets */
+        build_append_int_noprefix(table_data, isa_offset, 4);
+        if (cmo_offset) {
+            build_append_int_noprefix(table_data, cmo_offset, 4);
+        }
     }
 
     acpi_table_end(linker, &table);
-- 
2.39.2

Re: [PATCH 08/10] hw/riscv/virt-acpi-build.c: Add CMO information in RHCT

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> When CMO related extensions like Zicboz, Zicbom and Zicbop
> are enabled, the block size for those extensions need to be
> communicated via CMO node in RHCT. Add CMO node in RHCT if
> any of those CMO extensions are detected.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt-acpi-build.c | 64 +++++++++++++++++++++++++++++++++-----
>   1 file changed, 56 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 9f2d0c92b0..2d2bd3b970 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -146,6 +146,7 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>    * 5.2.36 RISC-V Hart Capabilities Table (RHCT)
>    * REF: https://github.com/riscv-non-isa/riscv-acpi/issues/16
>    *      https://drive.google.com/file/d/1nP3nFiH4jkPMp6COOxP6123DCZKR-tia/view
> + *      https://drive.google.com/file/d/1sKbOa8m1UZw1JkquZYe3F1zQBN1xXsaf/view
>    */
>   static void build_rhct(GArray *table_data,
>                          BIOSLinker *linker,
> @@ -155,8 +156,8 @@ static void build_rhct(GArray *table_data,
>       MachineState *ms = MACHINE(s);
>       const CPUArchIdList *arch_ids = mc->possible_cpu_arch_ids(ms);
>       size_t len, aligned_len;
> -    uint32_t isa_offset, num_rhct_nodes;
> -    RISCVCPU *cpu;
> +    uint32_t isa_offset, num_rhct_nodes, cmo_offset = 0;
> +    RISCVCPU *cpu = &s->soc[0].harts[0];
>       char *isa;
>   
>       AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
> @@ -172,6 +173,9 @@ static void build_rhct(GArray *table_data,
>   
>       /* ISA + N hart info */
>       num_rhct_nodes = 1 + ms->smp.cpus;
> +    if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
> +        num_rhct_nodes++;
> +    }
>   
>       /* Number of RHCT nodes*/
>       build_append_int_noprefix(table_data, num_rhct_nodes, 4);
> @@ -183,7 +187,6 @@ static void build_rhct(GArray *table_data,
>       isa_offset = table_data->len - table.table_offset;
>       build_append_int_noprefix(table_data, 0, 2);   /* Type 0 */
>   
> -    cpu = &s->soc[0].harts[0];
>       isa = riscv_isa_string(cpu);
>       len = 8 + strlen(isa) + 1;
>       aligned_len = (len % 2) ? (len + 1) : len;
> @@ -199,14 +202,59 @@ static void build_rhct(GArray *table_data,
>           build_append_int_noprefix(table_data, 0x0, 1);   /* Optional Padding */
>       }
>   
> +    /* CMO node */
> +    if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
> +        cmo_offset = table_data->len - table.table_offset;
> +        build_append_int_noprefix(table_data, 1, 2);    /* Type */
> +        build_append_int_noprefix(table_data, 10, 2);   /* Total Length */
> +        build_append_int_noprefix(table_data, 0x1, 2);  /* Revision */
> +        build_append_int_noprefix(table_data, 0, 1);    /* Reserved */
> +
> +        /* CBOM block size */
> +        if (cpu->cfg.cbom_blocksize) {
> +            build_append_int_noprefix(table_data,
> +                                      __builtin_ctz(cpu->cfg.cbom_blocksize),
> +                                      1);
> +        } else {
> +            build_append_int_noprefix(table_data, 0, 1);
> +        }
> +
> +        /* CBOP block size */
> +        build_append_int_noprefix(table_data, 0, 1);
> +
> +        /* CBOZ block size */
> +        if (cpu->cfg.cboz_blocksize) {
> +            build_append_int_noprefix(table_data,
> +                                      __builtin_ctz(cpu->cfg.cboz_blocksize),
> +                                      1);
> +        } else {
> +            build_append_int_noprefix(table_data, 0, 1);
> +        }
> +    }
> +
>       /* Hart Info Node */
>       for (int i = 0; i < arch_ids->len; i++) {
> +        len = 16;
> +        int num_offsets = 1;
>           build_append_int_noprefix(table_data, 0xFFFF, 2);  /* Type */
> -        build_append_int_noprefix(table_data, 16, 2);      /* Length */
> -        build_append_int_noprefix(table_data, 0x1, 2);     /* Revision */
> -        build_append_int_noprefix(table_data, 1, 2);    /* Number of offsets */
> -        build_append_int_noprefix(table_data, i, 4);    /* ACPI Processor UID */
> -        build_append_int_noprefix(table_data, isa_offset, 4); /* Offsets[0] */
> +
> +        /* Length */
> +        if (cmo_offset) {
> +            len += 4;
> +            num_offsets++;
> +        }
> +
> +        build_append_int_noprefix(table_data, len, 2);
> +        build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
> +        /* Number of offsets */
> +        build_append_int_noprefix(table_data, num_offsets, 2);
> +        build_append_int_noprefix(table_data, i, 4);   /* ACPI Processor UID */
> +
> +        /* Offsets */
> +        build_append_int_noprefix(table_data, isa_offset, 4);
> +        if (cmo_offset) {
> +            build_append_int_noprefix(table_data, cmo_offset, 4);
> +        }
>       }
>   
>       acpi_table_end(linker, &table);

[PATCH 09/10] hw/riscv/virt-acpi-build.c: Add MMU node in RHCT

[Sunil V L]

MMU type information is available via MMU node in RHCT.
Add this node in RHCT.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/virt-acpi-build.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 2d2bd3b970..25745eee4c 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -158,6 +158,8 @@ static void build_rhct(GArray *table_data,
     size_t len, aligned_len;
     uint32_t isa_offset, num_rhct_nodes, cmo_offset = 0;
     RISCVCPU *cpu = &s->soc[0].harts[0];
+    uint32_t mmu_offset = 0;
+    uint8_t satp_mode_max;
     char *isa;
 
     AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
@@ -177,6 +179,10 @@ static void build_rhct(GArray *table_data,
         num_rhct_nodes++;
     }
 
+    if (cpu->cfg.satp_mode.supported != 0) {
+        num_rhct_nodes++;
+    }
+
     /* Number of RHCT nodes*/
     build_append_int_noprefix(table_data, num_rhct_nodes, 4);
 
@@ -202,6 +208,26 @@ static void build_rhct(GArray *table_data,
         build_append_int_noprefix(table_data, 0x0, 1);   /* Optional Padding */
     }
 
+    /* MMU node structure */
+    if (cpu->cfg.satp_mode.supported != 0) {
+        satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
+        mmu_offset = table_data->len - table.table_offset;
+        build_append_int_noprefix(table_data, 1, 2);    /* Type */
+        build_append_int_noprefix(table_data, 8, 2);    /* Total Length */
+        build_append_int_noprefix(table_data, 0x1, 2);  /* Revision */
+        build_append_int_noprefix(table_data, 0, 1);    /* Reserved */
+        /* Virtual Address Scheme */
+        if (satp_mode_max == VM_1_10_SV57) {
+            build_append_int_noprefix(table_data, 2, 1);    /* Sv57 */
+        } else if (satp_mode_max == VM_1_10_SV48) {
+            build_append_int_noprefix(table_data, 1, 1);    /* Sv48 */
+        } else if (satp_mode_max == VM_1_10_SV39) {
+            build_append_int_noprefix(table_data, 0, 1);    /* Sv39 */
+        } else {
+            assert(1);
+        }
+    }
+
     /* CMO node */
     if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
         cmo_offset = table_data->len - table.table_offset;
@@ -244,6 +270,11 @@ static void build_rhct(GArray *table_data,
             num_offsets++;
         }
 
+        if (mmu_offset) {
+            len += 4;
+            num_offsets++;
+        }
+
         build_append_int_noprefix(table_data, len, 2);
         build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
         /* Number of offsets */
@@ -252,9 +283,14 @@ static void build_rhct(GArray *table_data,
 
         /* Offsets */
         build_append_int_noprefix(table_data, isa_offset, 4);
+
         if (cmo_offset) {
             build_append_int_noprefix(table_data, cmo_offset, 4);
         }
+
+        if (mmu_offset) {
+            build_append_int_noprefix(table_data, mmu_offset, 4);
+        }
     }
 
     acpi_table_end(linker, &table);
-- 
2.39.2

Re: [PATCH 09/10] hw/riscv/virt-acpi-build.c: Add MMU node in RHCT

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> MMU type information is available via MMU node in RHCT.
> Add this node in RHCT.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   hw/riscv/virt-acpi-build.c | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 2d2bd3b970..25745eee4c 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -158,6 +158,8 @@ static void build_rhct(GArray *table_data,
>       size_t len, aligned_len;
>       uint32_t isa_offset, num_rhct_nodes, cmo_offset = 0;
>       RISCVCPU *cpu = &s->soc[0].harts[0];
> +    uint32_t mmu_offset = 0;
> +    uint8_t satp_mode_max;
>       char *isa;
>   
>       AcpiTable table = { .sig = "RHCT", .rev = 1, .oem_id = s->oem_id,
> @@ -177,6 +179,10 @@ static void build_rhct(GArray *table_data,
>           num_rhct_nodes++;
>       }
>   
> +    if (cpu->cfg.satp_mode.supported != 0) {
> +        num_rhct_nodes++;
> +    }
> +
>       /* Number of RHCT nodes*/
>       build_append_int_noprefix(table_data, num_rhct_nodes, 4);
>   
> @@ -202,6 +208,26 @@ static void build_rhct(GArray *table_data,
>           build_append_int_noprefix(table_data, 0x0, 1);   /* Optional Padding */
>       }
>   
> +    /* MMU node structure */
> +    if (cpu->cfg.satp_mode.supported != 0) {
> +        satp_mode_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
> +        mmu_offset = table_data->len - table.table_offset;
> +        build_append_int_noprefix(table_data, 1, 2);    /* Type */
> +        build_append_int_noprefix(table_data, 8, 2);    /* Total Length */
> +        build_append_int_noprefix(table_data, 0x1, 2);  /* Revision */
> +        build_append_int_noprefix(table_data, 0, 1);    /* Reserved */
> +        /* Virtual Address Scheme */
> +        if (satp_mode_max == VM_1_10_SV57) {
> +            build_append_int_noprefix(table_data, 2, 1);    /* Sv57 */
> +        } else if (satp_mode_max == VM_1_10_SV48) {
> +            build_append_int_noprefix(table_data, 1, 1);    /* Sv48 */
> +        } else if (satp_mode_max == VM_1_10_SV39) {
> +            build_append_int_noprefix(table_data, 0, 1);    /* Sv39 */
> +        } else {
> +            assert(1);
> +        }
> +    }
> +
>       /* CMO node */
>       if (cpu->cfg.ext_icbom || cpu->cfg.ext_icboz) {
>           cmo_offset = table_data->len - table.table_offset;
> @@ -244,6 +270,11 @@ static void build_rhct(GArray *table_data,
>               num_offsets++;
>           }
>   
> +        if (mmu_offset) {
> +            len += 4;
> +            num_offsets++;
> +        }
> +
>           build_append_int_noprefix(table_data, len, 2);
>           build_append_int_noprefix(table_data, 0x1, 2); /* Revision */
>           /* Number of offsets */
> @@ -252,9 +283,14 @@ static void build_rhct(GArray *table_data,
>   
>           /* Offsets */
>           build_append_int_noprefix(table_data, isa_offset, 4);
> +
>           if (cmo_offset) {
>               build_append_int_noprefix(table_data, cmo_offset, 4);
>           }
> +
> +        if (mmu_offset) {
> +            build_append_int_noprefix(table_data, mmu_offset, 4);
> +        }
>       }
>   
>       acpi_table_end(linker, &table);

[PATCH 10/10] hw/riscv/virt-acpi-build.c: Add IO controllers and devices

[Sunil V L]

Add basic IO controllers and devices like PCI, VirtIO and UART
in the ACPI namespace.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 hw/riscv/Kconfig           |  1 +
 hw/riscv/virt-acpi-build.c | 87 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index b6a5eb4452..a50717be87 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -45,6 +45,7 @@ config RISCV_VIRT
     select FW_CFG_DMA
     select PLATFORM_BUS
     select ACPI
+    select ACPI_PCI
 
 config SHAKTI_C
     bool
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 25745eee4c..91f06fdc97 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -27,6 +27,7 @@
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/aml-build.h"
+#include "hw/acpi/pci.h"
 #include "hw/acpi/utils.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -35,6 +36,7 @@
 #include "hw/riscv/virt.h"
 #include "hw/riscv/numa.h"
 #include "hw/intc/riscv_aclint.h"
+#include "hw/pci-host/gpex.h"
 
 #define ACPI_BUILD_TABLE_SIZE             0x20000
 #define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
@@ -138,6 +140,55 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
     }
 }
 
+static void
+acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
+                    uint32_t uart_irq)
+{
+    Aml *dev = aml_device("COM0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(uart_memmap->base,
+                                         uart_memmap->size, AML_READ_WRITE));
+    aml_append(crs,
+                aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                               AML_EXCLUSIVE, &uart_irq, 1));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    Aml *pkg = aml_package(2);
+    aml_append(pkg, aml_string("clock-frequency"));
+    aml_append(pkg, aml_int(3686400));
+
+    Aml *UUID = aml_touuid("DAFFD814-6EBA-4D8C-8A91-BC9BBF4AA301");
+
+    Aml *pkg1 = aml_package(1);
+    aml_append(pkg1, pkg);
+
+    Aml *package = aml_package(2);
+    aml_append(package, UUID);
+    aml_append(package, pkg1);
+
+    aml_append(dev, aml_name_decl("_DSD", package));
+    aml_append(scope, dev);
+}
+
+static void
+acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
+                   uint32_t irq, RISCVVirtState *s)
+{
+    struct GPEXConfig cfg = {
+        .mmio32 = memmap[VIRT_PCIE_MMIO],
+        .mmio64 = memmap[VIRT_HIGH_PCIE_MMIO],
+        .pio = memmap[VIRT_PCIE_PIO],
+        .ecam = memmap[VIRT_PCIE_ECAM],
+        .irq = irq,
+        .bus = s->bus,
+    };
+
+    acpi_dsdt_add_gpex(scope, &cfg);
+}
+
 /* RHCT Node[N] starts at offset 56 */
 #define RHCT_NODE_ARRAY_OFFSET 56
 
@@ -318,6 +369,8 @@ static void build_dsdt(GArray *table_data,
                        RISCVVirtState *s)
 {
     Aml *scope, *dsdt;
+    MachineState *ms = MACHINE(s);
+    uint8_t socket_count;
     const MemMapEntry *memmap = s->memmap;
     AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = s->oem_id,
                         .oem_table_id = s->oem_table_id };
@@ -337,6 +390,30 @@ static void build_dsdt(GArray *table_data,
 
     acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
 
+    socket_count = riscv_socket_count(ms);
+
+    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
+
+    if (socket_count == 1) {
+        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
+                             VIRTIO_IRQ, VIRTIO_COUNT);
+        acpi_dsdt_add_pci(scope, memmap, PCIE_IRQ, s);
+    } else if (socket_count == 2) {
+        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
+                             VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
+                             VIRTIO_COUNT);
+        acpi_dsdt_add_pci(scope, memmap,
+                          PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
+                          s);
+    } else {
+        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
+                             VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
+                             VIRTIO_COUNT);
+        acpi_dsdt_add_pci(scope, memmap,
+                          PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES * 2,
+                          s);
+    }
+
     aml_append(dsdt, scope);
 
     /* copy AML table into ACPI tables blob and patch header there */
@@ -486,6 +563,16 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
     acpi_add_table(table_offsets, tables_blob);
     build_rhct(tables_blob, tables->linker, s);
 
+    acpi_add_table(table_offsets, tables_blob);
+    {
+        AcpiMcfgInfo mcfg = {
+           .base = s->memmap[VIRT_PCIE_MMIO].base,
+           .size = s->memmap[VIRT_PCIE_MMIO].size,
+        };
+        build_mcfg(tables_blob, tables->linker, &mcfg, s->oem_id,
+                   s->oem_table_id);
+    }
+
     /* XSDT is pointed to by RSDP */
     xsdt = tables_blob->len;
     build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,
-- 
2.39.2

Re: [PATCH 10/10] hw/riscv/virt-acpi-build.c: Add IO controllers and devices

[Daniel Henrique Barboza]

On 7/12/23 13:39, Sunil V L wrote:
> Add basic IO controllers and devices like PCI, VirtIO and UART
> in the ACPI namespace.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/Kconfig           |  1 +
>   hw/riscv/virt-acpi-build.c | 87 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 88 insertions(+)
> 
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index b6a5eb4452..a50717be87 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -45,6 +45,7 @@ config RISCV_VIRT
>       select FW_CFG_DMA
>       select PLATFORM_BUS
>       select ACPI
> +    select ACPI_PCI
>   
>   config SHAKTI_C
>       bool
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 25745eee4c..91f06fdc97 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -27,6 +27,7 @@
>   #include "hw/acpi/acpi-defs.h"
>   #include "hw/acpi/acpi.h"
>   #include "hw/acpi/aml-build.h"
> +#include "hw/acpi/pci.h"
>   #include "hw/acpi/utils.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
> @@ -35,6 +36,7 @@
>   #include "hw/riscv/virt.h"
>   #include "hw/riscv/numa.h"
>   #include "hw/intc/riscv_aclint.h"
> +#include "hw/pci-host/gpex.h"
>   
>   #define ACPI_BUILD_TABLE_SIZE             0x20000
>   #define ACPI_BUILD_INTC_ID(socket, index) ((socket << 24) | (index))
> @@ -138,6 +140,55 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s)
>       }
>   }
>   
> +static void
> +acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> +                    uint32_t uart_irq)
> +{
> +    Aml *dev = aml_device("COM0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0501")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(uart_memmap->base,
> +                                         uart_memmap->size, AML_READ_WRITE));
> +    aml_append(crs,
> +                aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                               AML_EXCLUSIVE, &uart_irq, 1));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    Aml *pkg = aml_package(2);
> +    aml_append(pkg, aml_string("clock-frequency"));
> +    aml_append(pkg, aml_int(3686400));
> +
> +    Aml *UUID = aml_touuid("DAFFD814-6EBA-4D8C-8A91-BC9BBF4AA301");
> +
> +    Aml *pkg1 = aml_package(1);
> +    aml_append(pkg1, pkg);
> +
> +    Aml *package = aml_package(2);
> +    aml_append(package, UUID);
> +    aml_append(package, pkg1);
> +
> +    aml_append(dev, aml_name_decl("_DSD", package));
> +    aml_append(scope, dev);
> +}
> +
> +static void
> +acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> +                   uint32_t irq, RISCVVirtState *s)
> +{
> +    struct GPEXConfig cfg = {
> +        .mmio32 = memmap[VIRT_PCIE_MMIO],
> +        .mmio64 = memmap[VIRT_HIGH_PCIE_MMIO],
> +        .pio = memmap[VIRT_PCIE_PIO],
> +        .ecam = memmap[VIRT_PCIE_ECAM],
> +        .irq = irq,
> +        .bus = s->bus,
> +    };
> +
> +    acpi_dsdt_add_gpex(scope, &cfg);
> +}
> +
>   /* RHCT Node[N] starts at offset 56 */
>   #define RHCT_NODE_ARRAY_OFFSET 56
>   
> @@ -318,6 +369,8 @@ static void build_dsdt(GArray *table_data,
>                          RISCVVirtState *s)
>   {
>       Aml *scope, *dsdt;
> +    MachineState *ms = MACHINE(s);
> +    uint8_t socket_count;
>       const MemMapEntry *memmap = s->memmap;
>       AcpiTable table = { .sig = "DSDT", .rev = 2, .oem_id = s->oem_id,
>                           .oem_table_id = s->oem_table_id };
> @@ -337,6 +390,30 @@ static void build_dsdt(GArray *table_data,
>   
>       acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
>   
> +    socket_count = riscv_socket_count(ms);
> +
> +    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
> +
> +    if (socket_count == 1) {
> +        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
> +                             VIRTIO_IRQ, VIRTIO_COUNT);
> +        acpi_dsdt_add_pci(scope, memmap, PCIE_IRQ, s);
> +    } else if (socket_count == 2) {
> +        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
> +                             VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
> +                             VIRTIO_COUNT);
> +        acpi_dsdt_add_pci(scope, memmap,
> +                          PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
> +                          s);
> +    } else {
> +        acpi_dsdt_add_virtio(scope, &memmap[VIRT_VIRTIO],
> +                             VIRTIO_IRQ + VIRT_IRQCHIP_NUM_SOURCES,
> +                             VIRTIO_COUNT);
> +        acpi_dsdt_add_pci(scope, memmap,
> +                          PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES * 2,
> +                          s);
> +    }
> +
>       aml_append(dsdt, scope);
>   
>       /* copy AML table into ACPI tables blob and patch header there */
> @@ -486,6 +563,16 @@ static void virt_acpi_build(RISCVVirtState *s, AcpiBuildTables *tables)
>       acpi_add_table(table_offsets, tables_blob);
>       build_rhct(tables_blob, tables->linker, s);
>   
> +    acpi_add_table(table_offsets, tables_blob);
> +    {
> +        AcpiMcfgInfo mcfg = {
> +           .base = s->memmap[VIRT_PCIE_MMIO].base,
> +           .size = s->memmap[VIRT_PCIE_MMIO].size,
> +        };
> +        build_mcfg(tables_blob, tables->linker, &mcfg, s->oem_id,
> +                   s->oem_table_id);
> +    }
> +
>       /* XSDT is pointed to by RSDP */
>       xsdt = tables_blob->len;
>       build_xsdt(tables_blob, tables->linker, table_offsets, s->oem_id,