Instantiate TYPE_ICH9_AHCI in TYPE_ICH9_SOUTHBRIDGE.
Since the PC machines can disable SATA (see the
PC_MACHINE_SATA dynamic property), add the 'sata-enabled'
property to disable it.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
MAINTAINERS | 2 ++
include/hw/southbridge/ich9.h | 4 ----
hw/i386/pc_q35.c | 25 ++++---------------------
hw/southbridge/ich9.c | 35 +++++++++++++++++++++++++++++++++++
hw/i386/Kconfig | 1 -
hw/southbridge/Kconfig | 1 +
6 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index d1a2eddd4c..937ebb5c96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2607,9 +2607,11 @@ M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
S: Supported
F: hw/acpi/ich9*.c
F: hw/i2c/smbus_ich9.c
+F: hw/ide/ich.c
F: hw/isa/lpc_ich9.c
F: hw/southbridge/ich9.c
F: include/hw/acpi/ich9*.h
+F: include/hw/ide/ahci-pci.h
F: include/hw/pci-bridge/ich_dmi_pci.h
F: include/hw/southbridge/ich9.h
diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
index b9122d299d..ac7f9f4ff5 100644
--- a/include/hw/southbridge/ich9.h
+++ b/include/hw/southbridge/ich9.h
@@ -166,10 +166,6 @@ struct ICH9LPCState {
#define ICH9_GPIO_GSI "gsi"
-/* D31:F2 SATA Controller #1 */
-#define ICH9_SATA1_DEV 31
-#define ICH9_SATA1_FUNC 2
-
/* D31:F0 power management I/O registers
offset from the address ICH9_LPC_PMBASE */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2f15af540f..060358d449 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -61,9 +61,6 @@
#include "hw/acpi/acpi.h"
#include "target/i386/cpu.h"
-/* ICH9 AHCI has 6 ports */
-#define MAX_SATA_PORTS 6
-
struct ehci_companions {
const char *name;
int func;
@@ -129,7 +126,7 @@ static void pc_q35_init(MachineState *machine)
PCIDevice *lpc;
Object *lpc_obj;
DeviceState *lpc_dev;
- BusState *idebus[MAX_SATA_PORTS];
+ BusState *idebus[2] = { };
ISADevice *rtc_state;
MemoryRegion *system_memory = get_system_memory();
MemoryRegion *system_io = get_system_io();
@@ -138,7 +135,6 @@ static void pc_q35_init(MachineState *machine)
ISABus *isa_bus;
int i;
ram_addr_t lowmem;
- DriveInfo *hd[MAX_SATA_PORTS];
MachineClass *mc = MACHINE_GET_CLASS(machine);
bool acpi_pcihp;
bool keep_pci_slot_hpc;
@@ -239,6 +235,7 @@ static void pc_q35_init(MachineState *machine)
object_property_set_link(OBJECT(ich9), "mch-pcie-bus",
OBJECT(host_bus), &error_abort);
qdev_prop_set_bit(ich9, "d2p-enabled", false);
+ qdev_prop_set_bit(ich9, "sata-enabled", pcms->sata_enabled);
qdev_realize_and_unref(ich9, NULL, &error_fatal);
/* irq lines */
@@ -302,22 +299,8 @@ static void pc_q35_init(MachineState *machine)
0xff0104);
if (pcms->sata_enabled) {
- PCIDevice *pdev;
- AHCIPCIState *ich9;
-
- /* ahci and SATA device, for q35 1 ahci controller is built-in */
- pdev = pci_create_simple_multifunction(host_bus,
- PCI_DEVFN(ICH9_SATA1_DEV,
- ICH9_SATA1_FUNC),
- "ich9-ahci");
- ich9 = ICH9_AHCI(pdev);
- idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
- idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
- g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
- ide_drive_get(hd, ich9->ahci.ports);
- ahci_ide_create_devs(&ich9->ahci, hd);
- } else {
- idebus[0] = idebus[1] = NULL;
+ idebus[0] = qdev_get_child_bus(ich9, "ide.0");
+ idebus[1] = qdev_get_child_bus(ich9, "ide.1");
}
if (machine_usb(machine)) {
diff --git a/hw/southbridge/ich9.c b/hw/southbridge/ich9.c
index 6df47e81fb..233dc1c5d7 100644
--- a/hw/southbridge/ich9.c
+++ b/hw/southbridge/ich9.c
@@ -13,22 +13,30 @@
#include "hw/southbridge/ich9.h"
#include "hw/pci/pci.h"
#include "hw/pci-bridge/ich_dmi_pci.h"
+#include "hw/ide/ahci-pci.h"
+#include "hw/ide.h"
#define ICH9_D2P_DEVFN PCI_DEVFN(30, 0)
+#define ICH9_SATA1_DEVFN PCI_DEVFN(31, 2)
+
+#define SATA_PORTS 6
struct ICH9State {
DeviceState parent_obj;
I82801b11Bridge d2p;
+ AHCIPCIState sata0;
PCIBus *pci_bus;
bool d2p_enabled;
+ bool sata_enabled;
};
static Property ich9_props[] = {
DEFINE_PROP_LINK("mch-pcie-bus", ICH9State, pci_bus,
TYPE_PCIE_BUS, PCIBus *),
DEFINE_PROP_BOOL("d2p-enabled", ICH9State, d2p_enabled, true),
+ DEFINE_PROP_BOOL("sata-enabled", ICH9State, sata_enabled, true),
DEFINE_PROP_END_OF_LIST(),
};
@@ -48,6 +56,29 @@ static bool ich9_realize_d2p(ICH9State *s, Error **errp)
return true;
}
+static bool ich9_realize_sata(ICH9State *s, Error **errp)
+{
+ DriveInfo *hd[SATA_PORTS];
+
+ object_initialize_child(OBJECT(s), "sata[0]", &s->sata0, TYPE_ICH9_AHCI);
+ qdev_prop_set_int32(DEVICE(&s->sata0), "addr", ICH9_SATA1_DEVFN);
+ if (!qdev_realize(DEVICE(&s->sata0), BUS(s->pci_bus), errp)) {
+ return false;
+ }
+ for (unsigned i = 0; i < SATA_PORTS; i++) {
+ g_autofree char *portname = g_strdup_printf("ide.%u", i);
+
+ object_property_add_alias(OBJECT(s), portname,
+ OBJECT(&s->sata0), portname);
+ }
+
+ g_assert(SATA_PORTS == s->sata0.ahci.ports);
+ ide_drive_get(hd, s->sata0.ahci.ports);
+ ahci_ide_create_devs(&s->sata0.ahci, hd);
+
+ return true;
+}
+
static void ich9_init(Object *obj)
{
}
@@ -64,6 +95,10 @@ static void ich9_realize(DeviceState *dev, Error **errp)
if (s->d2p_enabled && !ich9_realize_d2p(s, errp)) {
return;
}
+
+ if (s->sata_enabled && !ich9_realize_sata(s, errp)) {
+ return;
+ }
}
static void ich9_class_init(ObjectClass *klass, void *data)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d21638f4f9..226d7f6916 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -101,7 +101,6 @@ config Q35
select PCI_EXPRESS_Q35
select ICH9
select LPC_ICH9
- select AHCI_ICH9
select DIMM
select SMBIOS
select FW_CFG_DMA
diff --git a/hw/southbridge/Kconfig b/hw/southbridge/Kconfig
index db7259bf6f..f806033d48 100644
--- a/hw/southbridge/Kconfig
+++ b/hw/southbridge/Kconfig
@@ -4,3 +4,4 @@ config ICH9
bool
depends on PCI_EXPRESS
imply I82801B11
+ select AHCI_ICH9
--
2.41.0
On Mon, 19 Feb 2024, Philippe Mathieu-Daudé wrote:
> Instantiate TYPE_ICH9_AHCI in TYPE_ICH9_SOUTHBRIDGE.
>
> Since the PC machines can disable SATA (see the
> PC_MACHINE_SATA dynamic property), add the 'sata-enabled'
> property to disable it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> MAINTAINERS | 2 ++
> include/hw/southbridge/ich9.h | 4 ----
> hw/i386/pc_q35.c | 25 ++++---------------------
> hw/southbridge/ich9.c | 35 +++++++++++++++++++++++++++++++++++
> hw/i386/Kconfig | 1 -
> hw/southbridge/Kconfig | 1 +
> 6 files changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1a2eddd4c..937ebb5c96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2607,9 +2607,11 @@ M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> S: Supported
> F: hw/acpi/ich9*.c
> F: hw/i2c/smbus_ich9.c
> +F: hw/ide/ich.c
> F: hw/isa/lpc_ich9.c
> F: hw/southbridge/ich9.c
> F: include/hw/acpi/ich9*.h
> +F: include/hw/ide/ahci-pci.h
> F: include/hw/pci-bridge/ich_dmi_pci.h
> F: include/hw/southbridge/ich9.h
>
> diff --git a/include/hw/southbridge/ich9.h b/include/hw/southbridge/ich9.h
> index b9122d299d..ac7f9f4ff5 100644
> --- a/include/hw/southbridge/ich9.h
> +++ b/include/hw/southbridge/ich9.h
> @@ -166,10 +166,6 @@ struct ICH9LPCState {
>
> #define ICH9_GPIO_GSI "gsi"
>
> -/* D31:F2 SATA Controller #1 */
> -#define ICH9_SATA1_DEV 31
> -#define ICH9_SATA1_FUNC 2
> -
> /* D31:F0 power management I/O registers
> offset from the address ICH9_LPC_PMBASE */
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 2f15af540f..060358d449 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -61,9 +61,6 @@
> #include "hw/acpi/acpi.h"
> #include "target/i386/cpu.h"
>
> -/* ICH9 AHCI has 6 ports */
> -#define MAX_SATA_PORTS 6
> -
> struct ehci_companions {
> const char *name;
> int func;
> @@ -129,7 +126,7 @@ static void pc_q35_init(MachineState *machine)
> PCIDevice *lpc;
> Object *lpc_obj;
> DeviceState *lpc_dev;
> - BusState *idebus[MAX_SATA_PORTS];
> + BusState *idebus[2] = { };
> ISADevice *rtc_state;
> MemoryRegion *system_memory = get_system_memory();
> MemoryRegion *system_io = get_system_io();
> @@ -138,7 +135,6 @@ static void pc_q35_init(MachineState *machine)
> ISABus *isa_bus;
> int i;
> ram_addr_t lowmem;
> - DriveInfo *hd[MAX_SATA_PORTS];
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> bool acpi_pcihp;
> bool keep_pci_slot_hpc;
> @@ -239,6 +235,7 @@ static void pc_q35_init(MachineState *machine)
> object_property_set_link(OBJECT(ich9), "mch-pcie-bus",
> OBJECT(host_bus), &error_abort);
> qdev_prop_set_bit(ich9, "d2p-enabled", false);
> + qdev_prop_set_bit(ich9, "sata-enabled", pcms->sata_enabled);
> qdev_realize_and_unref(ich9, NULL, &error_fatal);
>
> /* irq lines */
> @@ -302,22 +299,8 @@ static void pc_q35_init(MachineState *machine)
> 0xff0104);
>
> if (pcms->sata_enabled) {
Shouldn't this condition be inverted if you only leave the else leg?
Regards,.
BALATON Zoltan
> - PCIDevice *pdev;
> - AHCIPCIState *ich9;
> -
> - /* ahci and SATA device, for q35 1 ahci controller is built-in */
> - pdev = pci_create_simple_multifunction(host_bus,
> - PCI_DEVFN(ICH9_SATA1_DEV,
> - ICH9_SATA1_FUNC),
> - "ich9-ahci");
> - ich9 = ICH9_AHCI(pdev);
> - idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
> - idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
> - g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
> - ide_drive_get(hd, ich9->ahci.ports);
> - ahci_ide_create_devs(&ich9->ahci, hd);
> - } else {
> - idebus[0] = idebus[1] = NULL;
> + idebus[0] = qdev_get_child_bus(ich9, "ide.0");
> + idebus[1] = qdev_get_child_bus(ich9, "ide.1");
> }
>
> if (machine_usb(machine)) {
> diff --git a/hw/southbridge/ich9.c b/hw/southbridge/ich9.c
> index 6df47e81fb..233dc1c5d7 100644
> --- a/hw/southbridge/ich9.c
> +++ b/hw/southbridge/ich9.c
> @@ -13,22 +13,30 @@
> #include "hw/southbridge/ich9.h"
> #include "hw/pci/pci.h"
> #include "hw/pci-bridge/ich_dmi_pci.h"
> +#include "hw/ide/ahci-pci.h"
> +#include "hw/ide.h"
>
> #define ICH9_D2P_DEVFN PCI_DEVFN(30, 0)
> +#define ICH9_SATA1_DEVFN PCI_DEVFN(31, 2)
> +
> +#define SATA_PORTS 6
>
> struct ICH9State {
> DeviceState parent_obj;
>
> I82801b11Bridge d2p;
> + AHCIPCIState sata0;
>
> PCIBus *pci_bus;
> bool d2p_enabled;
> + bool sata_enabled;
> };
>
> static Property ich9_props[] = {
> DEFINE_PROP_LINK("mch-pcie-bus", ICH9State, pci_bus,
> TYPE_PCIE_BUS, PCIBus *),
> DEFINE_PROP_BOOL("d2p-enabled", ICH9State, d2p_enabled, true),
> + DEFINE_PROP_BOOL("sata-enabled", ICH9State, sata_enabled, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -48,6 +56,29 @@ static bool ich9_realize_d2p(ICH9State *s, Error **errp)
> return true;
> }
>
> +static bool ich9_realize_sata(ICH9State *s, Error **errp)
> +{
> + DriveInfo *hd[SATA_PORTS];
> +
> + object_initialize_child(OBJECT(s), "sata[0]", &s->sata0, TYPE_ICH9_AHCI);
> + qdev_prop_set_int32(DEVICE(&s->sata0), "addr", ICH9_SATA1_DEVFN);
> + if (!qdev_realize(DEVICE(&s->sata0), BUS(s->pci_bus), errp)) {
> + return false;
> + }
> + for (unsigned i = 0; i < SATA_PORTS; i++) {
> + g_autofree char *portname = g_strdup_printf("ide.%u", i);
> +
> + object_property_add_alias(OBJECT(s), portname,
> + OBJECT(&s->sata0), portname);
> + }
> +
> + g_assert(SATA_PORTS == s->sata0.ahci.ports);
> + ide_drive_get(hd, s->sata0.ahci.ports);
> + ahci_ide_create_devs(&s->sata0.ahci, hd);
> +
> + return true;
> +}
> +
> static void ich9_init(Object *obj)
> {
> }
> @@ -64,6 +95,10 @@ static void ich9_realize(DeviceState *dev, Error **errp)
> if (s->d2p_enabled && !ich9_realize_d2p(s, errp)) {
> return;
> }
> +
> + if (s->sata_enabled && !ich9_realize_sata(s, errp)) {
> + return;
> + }
> }
>
> static void ich9_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index d21638f4f9..226d7f6916 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -101,7 +101,6 @@ config Q35
> select PCI_EXPRESS_Q35
> select ICH9
> select LPC_ICH9
> - select AHCI_ICH9
> select DIMM
> select SMBIOS
> select FW_CFG_DMA
> diff --git a/hw/southbridge/Kconfig b/hw/southbridge/Kconfig
> index db7259bf6f..f806033d48 100644
> --- a/hw/southbridge/Kconfig
> +++ b/hw/southbridge/Kconfig
> @@ -4,3 +4,4 @@ config ICH9
> bool
> depends on PCI_EXPRESS
> imply I82801B11
> + select AHCI_ICH9
>
On 19/2/24 19:31, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Philippe Mathieu-Daudé wrote:
>> Instantiate TYPE_ICH9_AHCI in TYPE_ICH9_SOUTHBRIDGE.
>>
>> Since the PC machines can disable SATA (see the
>> PC_MACHINE_SATA dynamic property), add the 'sata-enabled'
>> property to disable it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> MAINTAINERS | 2 ++
>> include/hw/southbridge/ich9.h | 4 ----
>> hw/i386/pc_q35.c | 25 ++++---------------------
>> hw/southbridge/ich9.c | 35 +++++++++++++++++++++++++++++++++++
>> hw/i386/Kconfig | 1 -
>> hw/southbridge/Kconfig | 1 +
>> 6 files changed, 42 insertions(+), 26 deletions(-)
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 2f15af540f..060358d449 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -61,9 +61,6 @@
>> #include "hw/acpi/acpi.h"
>> #include "target/i386/cpu.h"
>>
>> -/* ICH9 AHCI has 6 ports */
>> -#define MAX_SATA_PORTS 6
>> -
>> struct ehci_companions {
>> const char *name;
>> int func;
>> @@ -129,7 +126,7 @@ static void pc_q35_init(MachineState *machine)
>> PCIDevice *lpc;
>> Object *lpc_obj;
>> DeviceState *lpc_dev;
>> - BusState *idebus[MAX_SATA_PORTS];
>> + BusState *idebus[2] = { };
[*]
>> ISADevice *rtc_state;
>> MemoryRegion *system_memory = get_system_memory();
>> MemoryRegion *system_io = get_system_io();
>> @@ -138,7 +135,6 @@ static void pc_q35_init(MachineState *machine)
>> ISABus *isa_bus;
>> int i;
>> ram_addr_t lowmem;
>> - DriveInfo *hd[MAX_SATA_PORTS];
>> MachineClass *mc = MACHINE_GET_CLASS(machine);
>> bool acpi_pcihp;
>> bool keep_pci_slot_hpc;
>> @@ -239,6 +235,7 @@ static void pc_q35_init(MachineState *machine)
>> object_property_set_link(OBJECT(ich9), "mch-pcie-bus",
>> OBJECT(host_bus), &error_abort);
>> qdev_prop_set_bit(ich9, "d2p-enabled", false);
>> + qdev_prop_set_bit(ich9, "sata-enabled", pcms->sata_enabled);
>> qdev_realize_and_unref(ich9, NULL, &error_fatal);
>>
>> /* irq lines */
>> @@ -302,22 +299,8 @@ static void pc_q35_init(MachineState *machine)
>> 0xff0104);
>>
>> if (pcms->sata_enabled) {
>
> Shouldn't this condition be inverted if you only leave the else leg?
idebus[] is NULL-initialized in [*] so we can remove the else
ladder.
>
> Regards,.
> BALATON Zoltan
>
>> - PCIDevice *pdev;
>> - AHCIPCIState *ich9;
>> -
>> - /* ahci and SATA device, for q35 1 ahci controller is
>> built-in */
>> - pdev = pci_create_simple_multifunction(host_bus,
>> - PCI_DEVFN(ICH9_SATA1_DEV,
>> -
>> ICH9_SATA1_FUNC),
>> - "ich9-ahci");
>> - ich9 = ICH9_AHCI(pdev);
>> - idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
>> - idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
>> - g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
>> - ide_drive_get(hd, ich9->ahci.ports);
>> - ahci_ide_create_devs(&ich9->ahci, hd);
>> - } else {
>> - idebus[0] = idebus[1] = NULL;
>> + idebus[0] = qdev_get_child_bus(ich9, "ide.0");
>> + idebus[1] = qdev_get_child_bus(ich9, "ide.1");
>> }
© 2016 - 2024 Red Hat, Inc.