* [PATCH 0/7] PC cleanups
@ 2023-01-27 16:47 Bernhard Beschow
2023-01-27 16:47 ` [PATCH 1/7] hw/pci-host/i440fx: Inline sysbus_add_io() Bernhard Beschow
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
This series contains some random trivial cleanups I came across when working on
the PC machines. It consists of reducing the usage of global variables and
eliminating some redundancies.
Testing done:
* `make check`
* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
manjaro-kde-21.3.2-220704-linux515.iso`
* `qemu-system-x86_64 -M pc -m 2G -cdrom manjaro-kde-21.3.2-220704-linux515.iso`
Bernhard Beschow (7):
hw/pci-host/i440fx: Inline sysbus_add_io()
hw/pci-host/q35: Inline sysbus_add_io()
hw/i386/ich9: Rename Q35_MASK to ICH9_MASK
hw/i386/pc_q35: Resolve redundant q35_host variable
hw/i386/pc_{q35,piix}: Reuse MachineClass::desc as SMB product name
hw/i386/pc_{q35,piix}: Minimize usage of get_system_memory()
hw/i386/pc: Initialize ram_memory variable directly
include/hw/i386/ich9.h | 10 +++++-----
include/hw/i386/pc.h | 1 -
hw/i386/pc.c | 2 --
hw/i386/pc_piix.c | 8 ++++----
hw/i386/pc_q35.c | 34 ++++++++++++++++------------------
hw/pci-host/i440fx.c | 5 +++--
hw/pci-host/q35.c | 6 ++++--
7 files changed, 32 insertions(+), 34 deletions(-)
--
2.39.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/7] hw/pci-host/i440fx: Inline sysbus_add_io()
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 16:47 ` [PATCH 2/7] hw/pci-host/q35: " Bernhard Beschow
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
Rather than using get_system_io() as the parent memory region, use
s->bus->address_space_io which is set up as an alias in the pc machine.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/pci-host/i440fx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 262f82c303..9c6882d3fc 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -27,6 +27,7 @@
#include "qemu/range.h"
#include "hw/i386/pc.h"
#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
#include "hw/pci-host/i440fx.h"
#include "hw/qdev-properties.h"
@@ -217,10 +218,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
PCIHostState *s = PCI_HOST_BRIDGE(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
- sysbus_add_io(sbd, 0xcf8, &s->conf_mem);
+ memory_region_add_subregion(s->bus->address_space_io, 0xcf8, &s->conf_mem);
sysbus_init_ioports(sbd, 0xcf8, 4);
- sysbus_add_io(sbd, 0xcfc, &s->data_mem);
+ memory_region_add_subregion(s->bus->address_space_io, 0xcfc, &s->data_mem);
sysbus_init_ioports(sbd, 0xcfc, 4);
/* register i440fx 0xcf8 port as coalesced pio */
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/7] hw/pci-host/q35: Inline sysbus_add_io()
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
2023-01-27 16:47 ` [PATCH 1/7] hw/pci-host/i440fx: Inline sysbus_add_io() Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 16:47 ` [PATCH 3/7] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK Bernhard Beschow
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
Rather than using get_system_io() as the parent memory region, use
s->mch.address_space_io which is set up as an alias in the q35 machine.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/pci-host/q35.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 26390863d6..fa05844319 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
Q35PCIHost *s = Q35_HOST_DEVICE(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
- sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+ memory_region_add_subregion(s->mch.address_space_io,
+ MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
- sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+ memory_region_add_subregion(s->mch.address_space_io,
+ MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
/* register q35 0xcf8 port as coalesced pio */
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/7] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
2023-01-27 16:47 ` [PATCH 1/7] hw/pci-host/i440fx: Inline sysbus_add_io() Bernhard Beschow
2023-01-27 16:47 ` [PATCH 2/7] hw/pci-host/q35: " Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 16:47 ` [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
The Q35_MASK macro is already defined by TYPE_Q35_HOST_DEVICE, so let
TYPE_ICH9_LPC_DEVICE have its own one to prevent potential name
collisions.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/i386/ich9.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 222781e8b9..36e0ccd16a 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -75,7 +75,7 @@ struct ICH9LPCState {
qemu_irq gsi[GSI_NUM_PINS];
};
-#define Q35_MASK(bit, ms_bit, ls_bit) \
+#define ICH9_MASK(bit, ms_bit, ls_bit) \
((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
/* ICH9: Chipset Configuration Registers */
@@ -137,13 +137,13 @@ struct ICH9LPCState {
#define ICH9_LPC_NB_PIRQS 8 /* PCI A-H */
#define ICH9_LPC_PMBASE 0x40
-#define ICH9_LPC_PMBASE_BASE_ADDRESS_MASK Q35_MASK(32, 15, 7)
+#define ICH9_LPC_PMBASE_BASE_ADDRESS_MASK ICH9_MASK(32, 15, 7)
#define ICH9_LPC_PMBASE_RTE 0x1
#define ICH9_LPC_PMBASE_DEFAULT 0x1
#define ICH9_LPC_ACPI_CTRL 0x44
#define ICH9_LPC_ACPI_CTRL_ACPI_EN 0x80
-#define ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK Q35_MASK(8, 2, 0)
+#define ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK ICH9_MASK(8, 2, 0)
#define ICH9_LPC_ACPI_CTRL_9 0x0
#define ICH9_LPC_ACPI_CTRL_10 0x1
#define ICH9_LPC_ACPI_CTRL_11 0x2
@@ -162,7 +162,7 @@ struct ICH9LPCState {
#define ICH9_LPC_PIRQH_ROUT 0x6b
#define ICH9_LPC_PIRQ_ROUT_IRQEN 0x80
-#define ICH9_LPC_PIRQ_ROUT_MASK Q35_MASK(8, 3, 0)
+#define ICH9_LPC_PIRQ_ROUT_MASK ICH9_MASK(8, 3, 0)
#define ICH9_LPC_PIRQ_ROUT_DEFAULT 0x80
#define ICH9_LPC_GEN_PMCON_1 0xa0
@@ -172,7 +172,7 @@ struct ICH9LPCState {
#define ICH9_LPC_GEN_PMCON_LOCK 0xa6
#define ICH9_LPC_RCBA 0xf0
-#define ICH9_LPC_RCBA_BA_MASK Q35_MASK(32, 31, 14)
+#define ICH9_LPC_RCBA_BA_MASK ICH9_MASK(32, 31, 14)
#define ICH9_LPC_RCBA_EN 0x1
#define ICH9_LPC_RCBA_DEFAULT 0x0
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
` (2 preceding siblings ...)
2023-01-27 16:47 ` [PATCH 3/7] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 19:22 ` BALATON Zoltan
2023-01-27 16:47 ` [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name Bernhard Beschow
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
The variable is redundant to "phb" and is never used by its real type.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/i386/pc_q35.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 83c57c6eb1..dc94ce1081 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -118,7 +118,6 @@ static void pc_q35_init(MachineState *machine)
PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
X86MachineState *x86ms = X86_MACHINE(machine);
- Q35PCIHost *q35_host;
PCIHostState *phb;
PCIBus *host_bus;
PCIDevice *lpc;
@@ -206,10 +205,10 @@ static void pc_q35_init(MachineState *machine)
}
/* create pci host bus */
- q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
+ phb = PCI_HOST_BRIDGE(qdev_new(TYPE_Q35_HOST_DEVICE));
if (pcmc->pci_enabled) {
- pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
+ pci_hole64_size = object_property_get_uint(OBJECT(phb),
PCI_HOST_PROP_PCI_HOLE64_SIZE,
&error_abort);
}
@@ -218,22 +217,21 @@ static void pc_q35_init(MachineState *machine)
pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
pci_hole64_size);
- object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
- object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
+ object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
+ object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
OBJECT(ram_memory), NULL);
- object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
+ object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
OBJECT(pci_memory), NULL);
- object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
+ object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
OBJECT(get_system_memory()), NULL);
- object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_IO_MEM,
+ object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
OBJECT(system_io), NULL);
- object_property_set_int(OBJECT(q35_host), PCI_HOST_BELOW_4G_MEM_SIZE,
+ object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
x86ms->below_4g_mem_size, NULL);
- object_property_set_int(OBJECT(q35_host), PCI_HOST_ABOVE_4G_MEM_SIZE,
+ object_property_set_int(OBJECT(phb), PCI_HOST_ABOVE_4G_MEM_SIZE,
x86ms->above_4g_mem_size, NULL);
/* pci */
- sysbus_realize_and_unref(SYS_BUS_DEVICE(q35_host), &error_fatal);
- phb = PCI_HOST_BRIDGE(q35_host);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
host_bus = phb->bus;
/* create ISA bus */
lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
` (3 preceding siblings ...)
2023-01-27 16:47 ` [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 23:53 ` Philippe Mathieu-Daudé
2023-01-27 16:47 ` [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory() Bernhard Beschow
2023-01-27 16:47 ` [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly Bernhard Beschow
6 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
No need to repeat the descriptions.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..ee9d9a4175 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -197,7 +197,7 @@ static void pc_init1(MachineState *machine,
if (pcmc->smbios_defaults) {
MachineClass *mc = MACHINE_GET_CLASS(machine);
/* These values are guest ABI, do not change */
- smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
+ smbios_set_defaults("QEMU", mc->desc,
mc->name, pcmc->smbios_legacy_mode,
pcmc->smbios_uuid_encoded,
pcms->smbios_entry_point_type);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dc94ce1081..a97846ab9b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -198,7 +198,7 @@ static void pc_q35_init(MachineState *machine)
if (pcmc->smbios_defaults) {
/* These values are guest ABI, do not change */
- smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
+ smbios_set_defaults("QEMU", mc->desc,
mc->name, pcmc->smbios_legacy_mode,
pcmc->smbios_uuid_encoded,
pcms->smbios_entry_point_type);
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory()
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
` (4 preceding siblings ...)
2023-01-27 16:47 ` [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 19:30 ` BALATON Zoltan
2023-01-27 16:47 ` [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly Bernhard Beschow
6 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ee9d9a4175..5bde4533cc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine,
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
} else {
pci_bus = NULL;
- isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
+ isa_bus = isa_bus_new(NULL, system_memory, system_io,
&error_abort);
i8257_dma_init(isa_bus, 0);
pcms->hpet_enabled = false;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a97846ab9b..b97979bebb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine)
DeviceState *lpc_dev;
BusState *idebus[MAX_SATA_PORTS];
ISADevice *rtc_state;
+ MemoryRegion *system_memory = get_system_memory();
MemoryRegion *system_io = get_system_io();
MemoryRegion *pci_memory;
MemoryRegion *rom_memory;
@@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine)
rom_memory = pci_memory;
} else {
pci_memory = NULL;
- rom_memory = get_system_memory();
+ rom_memory = system_memory;
}
pc_guest_info_init(pcms);
@@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine)
}
/* allocate ram and load rom/bios */
- pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
+ pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
pci_hole64_size);
object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
@@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine)
object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
OBJECT(pci_memory), NULL);
object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
- OBJECT(get_system_memory()), NULL);
+ OBJECT(system_memory), NULL);
object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
OBJECT(system_io), NULL);
object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
` (5 preceding siblings ...)
2023-01-27 16:47 ` [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory() Bernhard Beschow
@ 2023-01-27 16:47 ` Bernhard Beschow
2023-01-27 23:57 ` Philippe Mathieu-Daudé
6 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-27 16:47 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
Bernhard Beschow
Going through pc_memory_init() seems quite complicated for a simple
assignment.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/i386/pc.h | 1 -
hw/i386/pc.c | 2 --
hw/i386/pc_piix.c | 4 ++--
hw/i386/pc_q35.c | 5 ++---
4 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 88a120bc23..5331b9a5c5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -163,7 +163,6 @@ void xen_load_linux(PCMachineState *pcms);
void pc_memory_init(PCMachineState *pcms,
MemoryRegion *system_memory,
MemoryRegion *rom_memory,
- MemoryRegion **ram_memory,
uint64_t pci_hole64_size);
uint64_t pc_pci_hole64_start(void);
DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e592bd969..8898cc9961 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -936,7 +936,6 @@ static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
void pc_memory_init(PCMachineState *pcms,
MemoryRegion *system_memory,
MemoryRegion *rom_memory,
- MemoryRegion **ram_memory,
uint64_t pci_hole64_size)
{
int linux_boot, i;
@@ -994,7 +993,6 @@ void pc_memory_init(PCMachineState *pcms,
* Split single memory region and use aliases to address portions of it,
* done for backwards compatibility with older qemus.
*/
- *ram_memory = machine->ram;
ram_below_4g = g_malloc(sizeof(*ram_below_4g));
memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", machine->ram,
0, x86ms->below_4g_mem_size);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5bde4533cc..00ba725656 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -143,6 +143,7 @@ static void pc_init1(MachineState *machine,
if (xen_enabled()) {
xen_hvm_init_pc(pcms, &ram_memory);
} else {
+ ram_memory = machine->ram;
if (!pcms->max_ram_below_4g) {
pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
}
@@ -205,8 +206,7 @@ static void pc_init1(MachineState *machine,
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory,
- rom_memory, &ram_memory, hole64_size);
+ pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
} else {
pc_system_flash_cleanup_unused(pcms);
if (machine->kernel_filename != NULL) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b97979bebb..8559924eb4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -128,7 +128,7 @@ static void pc_q35_init(MachineState *machine)
MemoryRegion *system_io = get_system_io();
MemoryRegion *pci_memory;
MemoryRegion *rom_memory;
- MemoryRegion *ram_memory;
+ MemoryRegion *ram_memory = machine->ram;
GSIState *gsi_state;
ISABus *isa_bus;
int i;
@@ -215,8 +215,7 @@ static void pc_q35_init(MachineState *machine)
}
/* allocate ram and load rom/bios */
- pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
- pci_hole64_size);
+ pc_memory_init(pcms, system_memory, rom_memory, pci_hole64_size);
object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
--
2.39.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable
2023-01-27 16:47 ` [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
@ 2023-01-27 19:22 ` BALATON Zoltan
2023-01-29 13:14 ` Bernhard Beschow
0 siblings, 1 reply; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-27 19:22 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
On Fri, 27 Jan 2023, Bernhard Beschow wrote:
> The variable is redundant to "phb" and is never used by its real type.
Also replace qdev_get_machine() with reference already passed to init
function. (Maybe worth mentioning in commit message even if too small
change for a separate patch.)
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/i386/pc_q35.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 83c57c6eb1..dc94ce1081 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -118,7 +118,6 @@ static void pc_q35_init(MachineState *machine)
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> X86MachineState *x86ms = X86_MACHINE(machine);
> - Q35PCIHost *q35_host;
> PCIHostState *phb;
The phb variable itself is only used once to get the bus from it and it's
mostly cast to Object * so maybe store it in a variable of that type to
remove most of the casts and IMO you can also remove PCIHostState *phb and
just use:
host_bus = PCI_HOST_BRIDGE(phost)->bus;
Regards,
BALATON Zoltan
> PCIBus *host_bus;
> PCIDevice *lpc;
> @@ -206,10 +205,10 @@ static void pc_q35_init(MachineState *machine)
> }
>
> /* create pci host bus */
> - q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
> + phb = PCI_HOST_BRIDGE(qdev_new(TYPE_Q35_HOST_DEVICE));
>
> if (pcmc->pci_enabled) {
> - pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
> + pci_hole64_size = object_property_get_uint(OBJECT(phb),
> PCI_HOST_PROP_PCI_HOLE64_SIZE,
> &error_abort);
> }
> @@ -218,22 +217,21 @@ static void pc_q35_init(MachineState *machine)
> pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> pci_hole64_size);
>
> - object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
> + object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
> OBJECT(ram_memory), NULL);
> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
> OBJECT(pci_memory), NULL);
> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
> OBJECT(get_system_memory()), NULL);
> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_IO_MEM,
> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
> OBJECT(system_io), NULL);
> - object_property_set_int(OBJECT(q35_host), PCI_HOST_BELOW_4G_MEM_SIZE,
> + object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
> x86ms->below_4g_mem_size, NULL);
> - object_property_set_int(OBJECT(q35_host), PCI_HOST_ABOVE_4G_MEM_SIZE,
> + object_property_set_int(OBJECT(phb), PCI_HOST_ABOVE_4G_MEM_SIZE,
> x86ms->above_4g_mem_size, NULL);
> /* pci */
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(q35_host), &error_fatal);
> - phb = PCI_HOST_BRIDGE(q35_host);
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
> host_bus = phb->bus;
> /* create ISA bus */
> lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory()
2023-01-27 16:47 ` [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory() Bernhard Beschow
@ 2023-01-27 19:30 ` BALATON Zoltan
2023-01-29 11:28 ` Bernhard Beschow
0 siblings, 1 reply; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-27 19:30 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
On Fri, 27 Jan 2023, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Why? I'd rather replace locals with direct call to function as it's not
expensive (just returns a global) and adding a local name to it is not
much shorter so why do that?
Regards,
BALATON Zoltan
> ---
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 7 ++++---
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ee9d9a4175..5bde4533cc 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine,
> isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> } else {
> pci_bus = NULL;
> - isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
> + isa_bus = isa_bus_new(NULL, system_memory, system_io,
> &error_abort);
> i8257_dma_init(isa_bus, 0);
> pcms->hpet_enabled = false;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index a97846ab9b..b97979bebb 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine)
> DeviceState *lpc_dev;
> BusState *idebus[MAX_SATA_PORTS];
> ISADevice *rtc_state;
> + MemoryRegion *system_memory = get_system_memory();
> MemoryRegion *system_io = get_system_io();
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> @@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine)
> rom_memory = pci_memory;
> } else {
> pci_memory = NULL;
> - rom_memory = get_system_memory();
> + rom_memory = system_memory;
> }
>
> pc_guest_info_init(pcms);
> @@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine)
> }
>
> /* allocate ram and load rom/bios */
> - pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> + pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
> pci_hole64_size);
>
> object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
> @@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine)
> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
> OBJECT(pci_memory), NULL);
> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
> - OBJECT(get_system_memory()), NULL);
> + OBJECT(system_memory), NULL);
> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
> OBJECT(system_io), NULL);
> object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name
2023-01-27 16:47 ` [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name Bernhard Beschow
@ 2023-01-27 23:53 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-27 23:53 UTC (permalink / raw)
To: Bernhard Beschow, qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
On 27/1/23 17:47, Bernhard Beschow wrote:
> No need to repeat the descriptions.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly
2023-01-27 16:47 ` [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly Bernhard Beschow
@ 2023-01-27 23:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-27 23:57 UTC (permalink / raw)
To: Bernhard Beschow, qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial,
open list:X86 Xen CPUs
On 27/1/23 17:47, Bernhard Beschow wrote:
> Going through pc_memory_init() seems quite complicated for a simple
> assignment.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/i386/pc.h | 1 -
> hw/i386/pc.c | 2 --
> hw/i386/pc_piix.c | 4 ++--
> hw/i386/pc_q35.c | 5 ++---
> 4 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 88a120bc23..5331b9a5c5 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -163,7 +163,6 @@ void xen_load_linux(PCMachineState *pcms);
> void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> MemoryRegion *rom_memory,
> - MemoryRegion **ram_memory,
> uint64_t pci_hole64_size);
> uint64_t pc_pci_hole64_start(void);
> DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6e592bd969..8898cc9961 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -936,7 +936,6 @@ static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
> void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> MemoryRegion *rom_memory,
> - MemoryRegion **ram_memory,
> uint64_t pci_hole64_size)
> {
> int linux_boot, i;
> @@ -994,7 +993,6 @@ void pc_memory_init(PCMachineState *pcms,
> * Split single memory region and use aliases to address portions of it,
> * done for backwards compatibility with older qemus.
> */
> - *ram_memory = machine->ram;
> ram_below_4g = g_malloc(sizeof(*ram_below_4g));
> memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", machine->ram,
> 0, x86ms->below_4g_mem_size);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 5bde4533cc..00ba725656 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -143,6 +143,7 @@ static void pc_init1(MachineState *machine,
> if (xen_enabled()) {
> xen_hvm_init_pc(pcms, &ram_memory);
> } else {
> + ram_memory = machine->ram;
> if (!pcms->max_ram_below_4g) {
> pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
> }
> @@ -205,8 +206,7 @@ static void pc_init1(MachineState *machine,
>
> /* allocate ram and load rom/bios */
> if (!xen_enabled()) {
> - pc_memory_init(pcms, system_memory,
> - rom_memory, &ram_memory, hole64_size);
> + pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
> } else {
> pc_system_flash_cleanup_unused(pcms);
> if (machine->kernel_filename != NULL) {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b97979bebb..8559924eb4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -128,7 +128,7 @@ static void pc_q35_init(MachineState *machine)
> MemoryRegion *system_io = get_system_io();
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> - MemoryRegion *ram_memory;
> + MemoryRegion *ram_memory = machine->ram;
> GSIState *gsi_state;
> ISABus *isa_bus;
> int i;
> @@ -215,8 +215,7 @@ static void pc_q35_init(MachineState *machine)
> }
>
> /* allocate ram and load rom/bios */
> - pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
> - pci_hole64_size);
> + pc_memory_init(pcms, system_memory, rom_memory, pci_hole64_size);
>
> object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory()
2023-01-27 19:30 ` BALATON Zoltan
@ 2023-01-29 11:28 ` Bernhard Beschow
0 siblings, 0 replies; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-29 11:28 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
Am 27. Januar 2023 19:30:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Fri, 27 Jan 2023, Bernhard Beschow wrote:
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
>Why? I'd rather replace locals with direct call to function as it's not expensive (just returns a global) and adding a local name to it is not much shorter so why do that?
The function has the assumption baked in to return a global which I'd like to factor out into a single variable assignment. This allows for easier experimentation with alternatives to global variables.
Moreover, extrating to a single variable mirrors how similar things are done in the pc and q35 machines.
Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> ---
>> hw/i386/pc_piix.c | 2 +-
>> hw/i386/pc_q35.c | 7 ++++---
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index ee9d9a4175..5bde4533cc 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine,
>> isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
>> } else {
>> pci_bus = NULL;
>> - isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
>> + isa_bus = isa_bus_new(NULL, system_memory, system_io,
>> &error_abort);
>> i8257_dma_init(isa_bus, 0);
>> pcms->hpet_enabled = false;
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index a97846ab9b..b97979bebb 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine)
>> DeviceState *lpc_dev;
>> BusState *idebus[MAX_SATA_PORTS];
>> ISADevice *rtc_state;
>> + MemoryRegion *system_memory = get_system_memory();
>> MemoryRegion *system_io = get_system_io();
>> MemoryRegion *pci_memory;
>> MemoryRegion *rom_memory;
>> @@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine)
>> rom_memory = pci_memory;
>> } else {
>> pci_memory = NULL;
>> - rom_memory = get_system_memory();
>> + rom_memory = system_memory;
>> }
>>
>> pc_guest_info_init(pcms);
>> @@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine)
>> }
>>
>> /* allocate ram and load rom/bios */
>> - pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
>> + pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
>> pci_hole64_size);
>>
>> object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
>> @@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine)
>> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
>> OBJECT(pci_memory), NULL);
>> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
>> - OBJECT(get_system_memory()), NULL);
>> + OBJECT(system_memory), NULL);
>> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
>> OBJECT(system_io), NULL);
>> object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable
2023-01-27 19:22 ` BALATON Zoltan
@ 2023-01-29 13:14 ` Bernhard Beschow
2023-01-29 14:37 ` BALATON Zoltan
0 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2023-01-29 13:14 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
Am 27. Januar 2023 19:22:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Fri, 27 Jan 2023, Bernhard Beschow wrote:
>> The variable is redundant to "phb" and is never used by its real type.
>
>Also replace qdev_get_machine() with reference already passed to init function. (Maybe worth mentioning in commit message even if too small change for a separate patch.)
Indeed, this can easily be missed. A separate patch allows for clean justification.
>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/i386/pc_q35.c | 22 ++++++++++------------
>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 83c57c6eb1..dc94ce1081 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -118,7 +118,6 @@ static void pc_q35_init(MachineState *machine)
>> PCMachineState *pcms = PC_MACHINE(machine);
>> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> X86MachineState *x86ms = X86_MACHINE(machine);
>> - Q35PCIHost *q35_host;
>> PCIHostState *phb;
>
>The phb variable itself is only used once to get the bus from it and it's mostly cast to Object * so maybe store it in a variable of that type to remove most of the casts and IMO you can also remove PCIHostState *phb and just use:
>
>host_bus = PCI_HOST_BRIDGE(phost)->bus;
Maybe one could also fish out the PCI bus using qdev_get_child_bus() like we do in pc_piix for the ISA bus already. Hm...
Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> PCIBus *host_bus;
>> PCIDevice *lpc;
>> @@ -206,10 +205,10 @@ static void pc_q35_init(MachineState *machine)
>> }
>>
>> /* create pci host bus */
>> - q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
>> + phb = PCI_HOST_BRIDGE(qdev_new(TYPE_Q35_HOST_DEVICE));
>>
>> if (pcmc->pci_enabled) {
>> - pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
>> + pci_hole64_size = object_property_get_uint(OBJECT(phb),
>> PCI_HOST_PROP_PCI_HOLE64_SIZE,
>> &error_abort);
>> }
>> @@ -218,22 +217,21 @@ static void pc_q35_init(MachineState *machine)
>> pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
>> pci_hole64_size);
>>
>> - object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
>> + object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
>> OBJECT(ram_memory), NULL);
>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
>> OBJECT(pci_memory), NULL);
>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
>> OBJECT(get_system_memory()), NULL);
>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_IO_MEM,
>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
>> OBJECT(system_io), NULL);
>> - object_property_set_int(OBJECT(q35_host), PCI_HOST_BELOW_4G_MEM_SIZE,
>> + object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
>> x86ms->below_4g_mem_size, NULL);
>> - object_property_set_int(OBJECT(q35_host), PCI_HOST_ABOVE_4G_MEM_SIZE,
>> + object_property_set_int(OBJECT(phb), PCI_HOST_ABOVE_4G_MEM_SIZE,
>> x86ms->above_4g_mem_size, NULL);
>> /* pci */
>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(q35_host), &error_fatal);
>> - phb = PCI_HOST_BRIDGE(q35_host);
>> + sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
>> host_bus = phb->bus;
>> /* create ISA bus */
>> lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable
2023-01-29 13:14 ` Bernhard Beschow
@ 2023-01-29 14:37 ` BALATON Zoltan
0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-29 14:37 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Michael S. Tsirkin, qemu-trivial
On Sun, 29 Jan 2023, Bernhard Beschow wrote:
> Am 27. Januar 2023 19:22:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Fri, 27 Jan 2023, Bernhard Beschow wrote:
>>> The variable is redundant to "phb" and is never used by its real type.
>>
>> Also replace qdev_get_machine() with reference already passed to init function. (Maybe worth mentioning in commit message even if too small change for a separate patch.)
>
> Indeed, this can easily be missed. A separate patch allows for clean justification.
I think just adding a sentence to commit message to clarify it is enough
no need to split it out but up to you.
Regards,
BALATON Zoltan
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> hw/i386/pc_q35.c | 22 ++++++++++------------
>>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index 83c57c6eb1..dc94ce1081 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -118,7 +118,6 @@ static void pc_q35_init(MachineState *machine)
>>> PCMachineState *pcms = PC_MACHINE(machine);
>>> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>>> X86MachineState *x86ms = X86_MACHINE(machine);
>>> - Q35PCIHost *q35_host;
>>> PCIHostState *phb;
>>
>> The phb variable itself is only used once to get the bus from it and it's mostly cast to Object * so maybe store it in a variable of that type to remove most of the casts and IMO you can also remove PCIHostState *phb and just use:
>>
>> host_bus = PCI_HOST_BRIDGE(phost)->bus;
>
> Maybe one could also fish out the PCI bus using qdev_get_child_bus() like we do in pc_piix for the ISA bus already. Hm...
>
> Best regards,
> Bernhard
>>
>> Regards,
>> BALATON Zoltan
>>
>>> PCIBus *host_bus;
>>> PCIDevice *lpc;
>>> @@ -206,10 +205,10 @@ static void pc_q35_init(MachineState *machine)
>>> }
>>>
>>> /* create pci host bus */
>>> - q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
>>> + phb = PCI_HOST_BRIDGE(qdev_new(TYPE_Q35_HOST_DEVICE));
>>>
>>> if (pcmc->pci_enabled) {
>>> - pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
>>> + pci_hole64_size = object_property_get_uint(OBJECT(phb),
>>> PCI_HOST_PROP_PCI_HOLE64_SIZE,
>>> &error_abort);
>>> }
>>> @@ -218,22 +217,21 @@ static void pc_q35_init(MachineState *machine)
>>> pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
>>> pci_hole64_size);
>>>
>>> - object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
>>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
>>> + object_property_add_child(OBJECT(machine), "q35", OBJECT(phb));
>>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_RAM_MEM,
>>> OBJECT(ram_memory), NULL);
>>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
>>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM,
>>> OBJECT(pci_memory), NULL);
>>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
>>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM,
>>> OBJECT(get_system_memory()), NULL);
>>> - object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_IO_MEM,
>>> + object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM,
>>> OBJECT(system_io), NULL);
>>> - object_property_set_int(OBJECT(q35_host), PCI_HOST_BELOW_4G_MEM_SIZE,
>>> + object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
>>> x86ms->below_4g_mem_size, NULL);
>>> - object_property_set_int(OBJECT(q35_host), PCI_HOST_ABOVE_4G_MEM_SIZE,
>>> + object_property_set_int(OBJECT(phb), PCI_HOST_ABOVE_4G_MEM_SIZE,
>>> x86ms->above_4g_mem_size, NULL);
>>> /* pci */
>>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(q35_host), &error_fatal);
>>> - phb = PCI_HOST_BRIDGE(q35_host);
>>> + sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
>>> host_bus = phb->bus;
>>> /* create ISA bus */
>>> lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
>>>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-01-29 14:40 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 16:47 [PATCH 0/7] PC cleanups Bernhard Beschow
2023-01-27 16:47 ` [PATCH 1/7] hw/pci-host/i440fx: Inline sysbus_add_io() Bernhard Beschow
2023-01-27 16:47 ` [PATCH 2/7] hw/pci-host/q35: " Bernhard Beschow
2023-01-27 16:47 ` [PATCH 3/7] hw/i386/ich9: Rename Q35_MASK to ICH9_MASK Bernhard Beschow
2023-01-27 16:47 ` [PATCH 4/7] hw/i386/pc_q35: Resolve redundant q35_host variable Bernhard Beschow
2023-01-27 19:22 ` BALATON Zoltan
2023-01-29 13:14 ` Bernhard Beschow
2023-01-29 14:37 ` BALATON Zoltan
2023-01-27 16:47 ` [PATCH 5/7] hw/i386/pc_{q35, piix}: Reuse MachineClass::desc as SMB product name Bernhard Beschow
2023-01-27 23:53 ` Philippe Mathieu-Daudé
2023-01-27 16:47 ` [PATCH 6/7] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory() Bernhard Beschow
2023-01-27 19:30 ` BALATON Zoltan
2023-01-29 11:28 ` Bernhard Beschow
2023-01-27 16:47 ` [PATCH 7/7] hw/i386/pc: Initialize ram_memory variable directly Bernhard Beschow
2023-01-27 23:57 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.