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

["Hao, Xudong" <xudong.hao@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, February 22, 2013 5:04 PM
> To: Hao, Xudong
> Cc: stefano.stabellini@xxxxxxxxxxxxx; Zhang, Xiantao; xen-devel@xxxxxxxxxxxxx;
> qemu-devel@xxxxxxxxxx; mst@xxxxxxxxxx; aliguori@xxxxxxxxxx
> 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 <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)?
> 
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


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