Re: [RFC PATCH-for-9.1 08/29] hw/i386/pc: Move CXLState to PcPciMachineState

2024-04-01 Thread Jonathan Cameron via
On Thu, 28 Mar 2024 16:54:16 +0100
Philippe Mathieu-Daudé  wrote:

> CXL depends on PCIe, which isn't available on non-PCI
> machines such the ISA-only PC one.
> Move CXLState to PcPciMachineState, and move the CXL
> specific calls to pc_pci_machine_initfn() and
> pc_pci_machine_done().
> 
> Signed-off-by: Philippe Mathieu-Daudé 

LGTM as a change on it's own - I've not reviewed the series
in general though, hence just an ack as an rb feels too strong.

Acked-by: Jonathan Cameron 




[RFC PATCH-for-9.1 08/29] hw/i386/pc: Move CXLState to PcPciMachineState

2024-03-28 Thread Philippe Mathieu-Daudé
CXL depends on PCIe, which isn't available on non-PCI
machines such the ISA-only PC one.
Move CXLState to PcPciMachineState, and move the CXL
specific calls to pc_pci_machine_initfn() and
pc_pci_machine_done().

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/pc.h |  3 ++-
 hw/i386/acpi-build.c | 14 +++---
 hw/i386/pc.c | 39 ---
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 24c8e17e62..a97493d29d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -54,13 +54,14 @@ typedef struct PCMachineState {
 hwaddr memhp_io_base;
 
 SGXEPCState sgx_epc;
-CXLState cxl_devices_state;
 } PCMachineState;
 
 typedef struct PcPciMachineState {
 PCMachineState parent_obj;
 
 Notifier machine_done;
+
+CXLState cxl_devices_state;
 } PcPciMachineState;
 
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b9890886f6..6e8e32e5d2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1554,6 +1554,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 crs_range_set_init(&crs_range_set);
 bus = PC_MACHINE(machine)->pcibus;
 if (bus) {
+PcPciMachineState *ppms;
+
+assert(pc_machine_is_pci_enabled(pcms));
+ppms = PC_PCI_MACHINE(machine);
+
 QLIST_FOREACH(bus, &bus->child, sibling) {
 uint8_t bus_num = pci_bus_num(bus);
 uint8_t numa_node = pci_bus_numa_node(bus);
@@ -1607,7 +1612,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 
 /* Handle the ranges for the PXB expanders */
 if (pci_bus_is_cxl(bus)) {
-MemoryRegion *mr = &pcms->cxl_devices_state.host_mr;
+MemoryRegion *mr = &ppms->cxl_devices_state.host_mr;
 uint64_t base = mr->addr;
 
 cxl_present = true;
@@ -2513,6 +2518,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 Object *vmgenid_dev;
 char *oem_id;
 char *oem_table_id;
+bool pci_enabled = pc_machine_is_pci_enabled(pcms);
+PcPciMachineState *ppms = pci_enabled ? PC_PCI_MACHINE(pcms) : NULL;
 
 acpi_get_pm_info(machine, &pm);
 acpi_get_misc_info(&misc);
@@ -2640,9 +2647,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
   machine->nvdimms_state, machine->ram_slots,
   x86ms->oem_id, x86ms->oem_table_id);
 }
-if (pcms->cxl_devices_state.is_enabled) {
+if (ppms && ppms->cxl_devices_state.is_enabled) {
 cxl_build_cedt(table_offsets, tables_blob, tables->linker,
-   x86ms->oem_id, x86ms->oem_table_id, 
&pcms->cxl_devices_state);
+   x86ms->oem_id, x86ms->oem_table_id,
+   &ppms->cxl_devices_state);
 }
 
 acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f9226f7115..6d87d1d4c2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -598,13 +598,6 @@ void pc_machine_done(Notifier *notifier, void *data)
 PCMachineState, machine_done);
 X86MachineState *x86ms = X86_MACHINE(pcms);
 
-cxl_hook_up_pxb_registers(pcms->pcibus, &pcms->cxl_devices_state,
-  &error_fatal);
-
-if (pcms->cxl_devices_state.is_enabled) {
-cxl_fmws_link_targets(&pcms->cxl_devices_state, &error_fatal);
-}
-
 /* set the number of CPUs */
 x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
@@ -626,6 +619,13 @@ static void pc_pci_machine_done(Notifier *notifier, void 
*data)
 PCMachineState *pcms = PC_MACHINE(ppms);
 X86MachineState *x86ms = X86_MACHINE(pcms);
 
+cxl_hook_up_pxb_registers(pcms->pcibus, &ppms->cxl_devices_state,
+  &error_fatal);
+
+if (ppms->cxl_devices_state.is_enabled) {
+cxl_fmws_link_targets(&ppms->cxl_devices_state, &error_fatal);
+}
+
 fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
 }
 
@@ -719,13 +719,14 @@ static uint64_t pc_get_cxl_range_start(PCMachineState 
*pcms)
 
 static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
 {
+PcPciMachineState *ppms = PC_PCI_MACHINE(pcms);
 uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
 
-if (pcms->cxl_devices_state.fixed_windows) {
+if (ppms->cxl_devices_state.fixed_windows) {
 GList *it;
 
 start = ROUND_UP(start, 256 * MiB);
-for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
+for (it = ppms->cxl_devices_state.fixed_windows; it; it = it->next) {
 CXLFixedWindow *fw = it->data;
 start += fw->size;
 }
@@ -823,6 +824,7 @@ void pc_memory_init(PCMachineState *pcms,
 hwaddr cxl_base, cxl_resv_end = 0;
 X86CPU *cpu = X86_CPU(first_cpu);
 bool pci_enabled = pc_machine_is_pci_enabled(pc