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

[Andreas Färber <afaerber@xxxxxxx>]

Am 22.02.2013 16:37, schrieb Hao, Xudong:
>> -----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.

Your colleague David Woodhouse has just prepared some i440fx cleanups.
Please use dev->config instead of the indirect d->dev.config.

Given Jan's endianness comment, would alignment allow you to simply
write as follows?

uint32_t addr = xen_enabled() ? 0xe0000000 : 0xf0000000;
*(uint32_t *)&dev->config[I440FX_PCI_HOLE_BASE] = cpu_to_le32(addr);

Then the byte-swapping would be explicit and the address in one piece
grep'able. Alternatively:

cpu_to_le32s(&addr);
for (i = 0; i < 4; i++) {
    dev->config[... + i] = ((uint8_t *)&addr)[i];
}

Anyway, please use "piix: " in the subject rather than "qemu: " if this
is supposed to go into upstream qemu.git.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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