Re: [Xen-devel] [PATCH] qemu: define a TOM register to report the base of PCI

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.02.13 at 04:23, Xudong Hao <xudong.hao@xxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel