[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [PATCH] qemu: define a TOM register to repo
From: |
Hao, Xudong |
Subject: |
Re: [Qemu-devel] [Xen-devel] [PATCH] qemu: define a TOM register to report the base of PCI |
Date: |
Fri, 22 Feb 2013 15:37:52 +0000 |
> -----Original Message-----
> From: Jan Beulich [mailto:address@hidden
> Sent: Friday, February 22, 2013 5:04 PM
> To: Hao, Xudong
> Cc: address@hidden; Zhang, Xiantao; address@hidden;
> address@hidden; address@hidden; address@hidden
> Subject: Re: [Xen-devel] [PATCH] qemu: define a TOM register to report the
> base of PCI
>
> >>> On 22.02.13 at 04:23, Xudong Hao <address@hidden> 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)?
>
Jan,
Here it should be little endian since the following config reading is little
endian, I'll correct it. Thanks your comments.
> 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.
Hvmloader did 64k-aligned, I'm not quite understand, could you help to point
out?
If considering wasting of config space, actually hvmloader only write 4 values
in this register: 3.75G(0xf0000000), 3.5G(0xe0000000), 3G(0xc0000000),
2G(0x80000000), then 1 byte is enough for xen.
> That would come closer to how real chipsets normally implement
> things like this.
>
> Jan