Am 20.02.2013 17:17, schrieb Andreas Färber: > Am 20.02.2013 16:35, schrieb David Woodhouse: >> This implements reset functionality for the i440FX, resetting all the >> PAM registers to their power-on defaults of no RAM access and thus >> forwarding all access to the 0xc0000-0xfffff range to PCI address space >> (i.e. to the actual ROM) instead of RAM. >> >> Fixing this is sufficient to work around a KVM bug which causes it to >> run 16-bit code at 0xffff0 instead of 0xfffffff0 on CPU reset. If reset >> was working correctly on the i440FX, that KVM bug wouldn't have >> *mattered* because the two addresses would have identical contents. >> >> There's been much discussion about the distinction between hard reset >> and soft reset, and the fact that many of our reset triggers (such as >> the keyboard controller and triple-fault handler) are actually doing a >> full system-wide hard reset when in fact they should be triggering >> something much more limited in scope. >> >> This patch exacerbates that existing problem only slightly, by causing >> the offending triggers to reset yet another piece of hardware that they >> shouldn't have been resetting. But the problem is largely theoretical >> anyway; mostly limited to 80286 protected mode software which needs to >> initiate a CPU reset to get back into real mode, but which *doesn't* >> want a full system reset. Such software is almost certainly already >> broken under Qemu anyway, because of all the *other* aspects of a full >> hard reset that are already happening. >> >> So this patch can be applied separately from any longer-term fixes to >> make the 'soft' reset actually do the right thing. >> >> Signed-off-by: David Woodhouse <[email protected]> >> >> diff --git a/hw/piix_pci.c b/hw/piix_pci.c >> index 6c77e49..f81c6aa 100644 >> --- a/hw/piix_pci.c >> +++ b/hw/piix_pci.c >> @@ -171,6 +171,24 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, >> int version_id) >> return 0; >> } >> >> +static void i440fx_reset(DeviceState *ds) >> +{ >> + PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, ds); >> + PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev); > > Please don't use DO_UPCAST() on QOM objects, there's QOM cast macros for > this, like PCI_DEVICE(). If there's one missing for PCII440FXState (I > think I only added them for the PHB itself?), it's two lines to add. > >> + uint8_t *pci_conf = d->dev.config;
In the same spirit that should be dev->config, not d->dev.config BTW. Cf. http://wiki.qemu.org/QOMConventions Andreas >> + >> + pci_conf[0x59] = 0x00; // Reset PAM setup > > scripts/checkpatch.pl will complain about this comment style. > >> + pci_conf[0x5a] = 0x00; >> + pci_conf[0x5b] = 0x00; >> + pci_conf[0x5c] = 0x00; >> + pci_conf[0x5d] = 0x00; >> + pci_conf[0x5e] = 0x00; >> + pci_conf[0x5f] = 0x00; >> + pci_conf[0x72] = 0x02; // And SMM >> + >> + i440fx_update_memory_mappings(d); >> +} >> + >> static int i440fx_post_load(void *opaque, int version_id) >> { >> PCII440FXState *d = opaque; >> @@ -615,6 +633,7 @@ static void i440fx_class_init(ObjectClass *klass, void >> *data) >> dc->desc = "Host bridge"; >> dc->no_user = 1; >> dc->vmsd = &vmstate_i440fx; >> + dc->reset = i440fx_reset; >> } >> >> static const TypeInfo i440fx_info = { > > Otherwise looks okay. > > Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
