Re: [Qemu-devel] Re: [PATCH] pci: initialize header type register.

[Michael S. Tsirkin]

On Mon, Feb 08, 2010 at 02:32:26PM -0600, Anthony Liguori wrote:
> On 02/08/2010 02:19 PM, Michael S. Tsirkin wrote:
>> On Mon, Feb 08, 2010 at 08:55:58PM +0100, Gerd Hoffmann wrote:
>>    
>>>    Hi,
>>>
>>>      
>>>> This still means we have two copies of same data
>>>> and need to maintain code that keeps them in sync,
>>>> even if that is called just at init time.
>>>>        
>>> No.  There is nothing to keep in sync.  And there is no extra copy of data.
>>>
>>> Today you have pci_set_*() calls somewhere in PCIDeviceInfo->init().
>>> I'd like to see them replaced with PCIDeviceInfo->$field + setup in
>>> common code.  The information that device $foo has vendor id 42 and
>>> device id 4711 (and other properties) just moves from code to data.
>>>      
>> We still need it in config array which is read by guest.
>> So that is two places.
>>    
>
> There's no reason that we couldn't make the config space read like all  
> of the other spaces we support.  IOW, instead of using an array to store  
> the data, store each element in a structure, and have a big switch().
>
> I'm not sure one's better than the other though TBH.

Yea. So the solution that needs less code is better.

> I think just universally moving to a set of accessors that took a  
> PCIDevice as an argument in the form of pci_device_set_vendor() would be  
> a big improvement.
>
> Regards,
>
> Anthony Liguori

Not sure it's such a *big* improvement, but I won't object to that.

-- 
MST