On Thu, 24 Jun 2021, Philippe Mathieu-Daudé wrote:
On 6/24/21 6:16 PM, Philippe Mathieu-Daudé wrote:On 6/24/21 6:01 PM, Philippe Mathieu-Daudé wrote:On 6/24/21 5:46 PM, Philippe Mathieu-Daudé wrote:Hi Zoltan,On 2/21/21 3:34 PM, Philippe Mathieu-Daudé wrote:From: BALATON Zoltan <bala...@eik.bme.hu> The base address of the SMBus io ports and its enabled status is set by registers in the PCI config space but this was not correctly emulated. Instead the SMBus registers were mapped on realize to the base address set by a property to the address expected by fuloong2e firmware. Fix the base and config register handling to more closely model hardware which allows to remove the property and allows the guest to control this mapping. Do all this in reset instead of realize so it's correctly updated on reset.This commit broken running PMON on Fuloong2E: https://www.mail-archive.com/qemu-devel@nongnu.org/msg752605.html console: PMON2000 MIPS Initializing. Standby... console: ERRORPC=00000000 CONFIG=00030932 console: PRID=00006302 console: DIMM read console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff console: 000000ff ... From here the console loops displaying this value...Tracing: mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80490 value 0xeee1 size 4 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe804d2 value 0x1 size 2 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80404 value 0x1 size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80004 value 0x7 size 4 mr_ops_read mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80081 value 0x0 size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80081 value 0x80 size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80083 value 0x89 size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80085 value 0x3 size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe8005a value 0x7 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xe2 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xe3 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xe6 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xe7 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xe8 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0x3f0 value 0xee size 1 mr_ops_write mr 0x5583912b2e00 (south-bridge-pci-config) addr 0x1fe80085 value 0x1 size 1These are: pci_cfg_write vt82c686b-pm 05:4 @0x90 <- 0xeee1Offset 93-90 – SMBus I/O Base ....................................... RW 15-4 I/O Base (16-byte I/O space)................ default = 00h pci_cfg_write vt82c686b-pm 05:4 @0x90 <- 0xeee1pci_cfg_write vt82c686b-pm 05:4 @0xd0 <- 0x1Offset D2 – SMBus Host Configuration ......................... RW SMBus Host Controller Enable 0 Disable SMB controller functions ......... default 1 Enable SMB controller functions pci_cfg_write vt82c686b-pm 05:4 @0xd0 <- 0x1 Hmm the datasheet indeed document 0xd2... why is the guest accessing 0xd0 to enable the function? It seems this is the problem, since if I replace d2 -> d0 PMON boots. See below [*].pci_cfg_write vt82c686b-pm 05:4 @0x4 <- 0x1(this one is PCI_COMMAND)pci_cfg_write vt82c686b-isa 05:0 @0x4 <- 0x7 pci_cfg_read vt82c686b-isa 05:0 @0x80 -> 0x0 pci_cfg_write vt82c686b-isa 05:0 @0x80 <- 0x80 pci_cfg_write vt82c686b-isa 05:0 @0x80 <- 0x89 pci_cfg_write vt82c686b-isa 05:0 @0x84 <- 0x3 pci_cfg_write vt82c686b-isa 05:0 @0x58 <- 0x7 pci_cfg_write vt82c686b-isa 05:0 @0x84 <- 0x1mr_ops_write mr 0x558390ff4d00 (io) addr 0xeee4 value 0xa1 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0xeee3 value 0x0 size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0xeee2 value 0x8 size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0xeee0 value 0x1f size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee2 value 0xffffffffffffffff size 1 mr_ops_write mr 0x558390ff4d00 (io) addr 0xeee2 value 0xff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1 mr_ops_read mr 0x558390ff4d00 (io) addr 0xeee0 value 0xffffffffffffffff size 1Expected: console: PMON2000 MIPS Initializing. Standby... console: ERRORPC=00000000 CONFIG=00030932 console: PRID=00006302 console: DIMM read console: 00000080 console: read memory type console: read number of rows ...Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> Reviewed-by: Jiaxun Yang <jiaxun.y...@flygoat.com> Message-Id: <f2ca2ad5f08ba8cee07afd9d67b4e75cda21db09.1610223397.git.bala...@eik.bme.hu> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> --- hw/isa/vt82c686.c | 49 +++++++++++++++++++++++++++++++++------------ hw/mips/fuloong2e.c | 4 +--- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index fe8961b0573..9c4d1530225 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -22,6 +22,7 @@ #include "hw/i2c/pm_smbus.h" #include "qapi/error.h" #include "qemu/module.h" +#include "qemu/range.h" #include "qemu/timer.h" #include "exec/address-spaces.h" #include "trace.h" @@ -34,7 +35,6 @@ struct VT686PMState { ACPIREGS ar; APMState apm; PMSMBus smb; - uint32_t smb_io_base; }; static void pm_io_space_update(VT686PMState *s) @@ -50,11 +50,22 @@ static void pm_io_space_update(VT686PMState *s) memory_region_transaction_commit(); } +static void smb_io_space_update(VT686PMState *s) +{ + uint32_t smbase = pci_get_long(s->dev.config + 0x90) & 0xfff0UL; + + memory_region_transaction_begin(); + memory_region_set_address(&s->smb.io, smbase); + memory_region_set_enabled(&s->smb.io, s->dev.config[0xd2] & BIT(0)); + memory_region_transaction_commit(); +} + static int vmstate_acpi_post_load(void *opaque, int version_id) { VT686PMState *s = opaque; pm_io_space_update(s); + smb_io_space_update(s); return 0; } @@ -77,8 +88,18 @@ static const VMStateDescription vmstate_acpi = { static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len) { + VT686PMState *s = VT82C686B_PM(d); + trace_via_pm_write(addr, val, len); pci_default_write_config(d, addr, val, len); + if (ranges_overlap(addr, len, 0x90, 4)) { + uint32_t v = pci_get_long(s->dev.config + 0x90); + pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1); + } + if (range_covers_byte(addr, len, 0xd2)) { + s->dev.config[0xd2] &= 0xf; + smb_io_space_update(s);[*] So the guest writing at 0xd0, this block is skipped, the I/O region never enabled.
Could it be it does word or dword i/o to access multiple addresses at once. Wasn't there a recent change to memory regions that could break this? Is adjusting valid access sizes to the mem region ops needed now to have the memory region handle this?
Regards, BALATON Zoltan
+ } } static void pm_update_sci(VT686PMState *s) @@ -103,6 +124,17 @@ static void pm_tmr_timer(ACPIREGS *ar) pm_update_sci(s); } +static void vt82c686b_pm_reset(DeviceState *d) +{ + VT686PMState *s = VT82C686B_PM(d); + + /* SMBus IO base */ + pci_set_long(s->dev.config + 0x90, 1); + s->dev.config[0xd2] = 0; + + smb_io_space_update(s); +} + static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp) { VT686PMState *s = VT82C686B_PM(dev); @@ -116,13 +148,9 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp) /* 0x48-0x4B is Power Management I/O Base */ pci_set_long(pci_conf + 0x48, 0x00000001); - /* SMB ports:0xeee0~0xeeef */ - s->smb_io_base = ((s->smb_io_base & 0xfff0) + 0x0); - pci_conf[0x90] = s->smb_io_base | 1; - pci_conf[0x91] = s->smb_io_base >> 8; - pci_conf[0xd2] = 0x90; pm_smbus_init(DEVICE(s), &s->smb, false); - memory_region_add_subregion(get_system_io(), s->smb_io_base, &s->smb.io); + memory_region_add_subregion(pci_address_space_io(dev), 0, &s->smb.io); + memory_region_set_enabled(&s->smb.io, false); apm_init(dev, &s->apm, NULL, s); @@ -135,11 +163,6 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp) acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2); } -static Property via_pm_properties[] = { - DEFINE_PROP_UINT32("smb_io_base", VT686PMState, smb_io_base, 0), - DEFINE_PROP_END_OF_LIST(), -}; - static void via_pm_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -151,10 +174,10 @@ static void via_pm_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_VIA_ACPI; k->class_id = PCI_CLASS_BRIDGE_OTHER; k->revision = 0x40; + dc->reset = vt82c686b_pm_reset; dc->desc = "PM"; dc->vmsd = &vmstate_acpi; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); - device_class_set_props(dc, via_pm_properties); } static const TypeInfo via_pm_info = { diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c index 1f3680fda3e..94f4718147f 100644 --- a/hw/mips/fuloong2e.c +++ b/hw/mips/fuloong2e.c @@ -230,9 +230,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc, pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci"); pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci"); - dev = pci_new(PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM); - qdev_prop_set_uint32(DEVICE(dev), "smb_io_base", 0xeee1); - pci_realize_and_unref(dev, pci_bus, &error_fatal); + dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM); *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c")); /* Audio support */
