>>> On 22.02.13 at 04:23, Xudong Hao <xudong....@intel.com> wrote: > @@ -203,6 +251,16 @@ static int i440fx_initfn(PCIDevice *dev) > > d->dev.config[I440FX_SMRAM] = 0x02; > > + /* Emulate top of memory, here use 0xe0000000 as default val*/ > + if (xen_enabled()) { > + d->dev.config[I440FX_PCI_HOLE_BASE] = 0xf0; > + } else { > + d->dev.config[I440FX_PCI_HOLE_BASE] = 0xe0; > + } > + d->dev.config[I440FX_PCI_HOLE_BASE + 1] = 0x00; > + d->dev.config[I440FX_PCI_HOLE_BASE + 2] = 0x00; > + d->dev.config[I440FX_PCI_HOLE_BASE + 3] = 0x00; > + > cpu_smm_register(&i440fx_set_smm, d); > return 0; > }
Isn't this the wrong way round (big endian, when it needs to be little)? Or else, this read and calculation >+ pci_hole_start = pci_default_read_config(&f->dev, I440FX_PCI_HOLE_BASE, >4); >+ pci_hole_size = 0x100000000ULL - pci_hole_start; would seem wrong (e.g. if the granularity is meant to be 16M). And then again, wasting 4 bytes of config space on something that one ought to be allowed to expect to be at least 64k-aligned seems questionable too. hvmloader surely could align the value up to the next 64k boundary before writing the - then only word size - field. That would come closer to how real chipsets normally implement things like this. Jan