qemu-devel
[Top][All Lists]
Advanced

[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: Andreas Färber
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] qemu: define a TOM register to report the base of PCI
Date: Fri, 22 Feb 2013 19:52:26 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2

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

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]