Re: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device

Pekka Enberg
2011-12-06T13:29:00Z

On Tue, 6 Dec 2011, Cyrill Gorcunov wrote:
> On Tue, Dec 06, 2011 at 01:58:24PM +0200, Pekka Enberg wrote:
>> On Tue, 2011-12-06 at 15:47 +0400, Cyrill Gorcunov wrote:
>>> On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
>>>> On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>>>>> On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
>>>>>> On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans <matt@ozlabs.org> wrote:
>>>>>>> vesa, pci-shmem and virtio-pci devices need to set up config space with
>>>>>>> little-endian conversions (as config space is LE). The pci_config_address
>>>>>>> bitfield also needs to be reversed when building on BE systems.
>>>>>>>
>>>>>>> Signed-off-by: Matt Evans <matt@ozlabs.org>
>>>>>>
>>>>>> Looks OK to me. Sasha, Cyrill?
>>>>>>
>>>>>
>>>>> BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
>>>>
>>>> Hmm. This seems to break "make check" for me:
>>>>
>>>
>>> If you change back to
>>>
>>> static struct pci_device_header pci_shmem_pci_device = {
>>> ...
>>> .class = 0xFF0000, /* misc pci device */
>>> ...
>>> };
>>>
>>> does it help?
>>
>> No but dropping these hunks fixes it for me:
>>
>> @@ -17,7 +18,8 @@
>> #define PCI_CONFIG_BUS_FORWARD 0xcfa
>> #define PCI_IO_SIZE 0x100
>>
>> -struct pci_config_address {
>> +union pci_config_address {
>> +#if __BYTE_ORDER = __LITTLE_ENDIAN
>> unsigned zeros : 2; /* 1 .. 0 */
>> unsigned register_number : 6; /* 7 .. 2 */
>> unsigned function_number : 3; /* 10 .. 8 */
>> @@ -25,6 +27,16 @@ struct pci_config_address {
>> unsigned bus_number : 8; /* 23 .. 16 */
>> unsigned reserved : 7; /* 30 .. 24 */
>> unsigned enable_bit : 1; /* 31 */
>> +#else
>> + unsigned enable_bit : 1; /* 31 */
>> + unsigned reserved : 7; /* 30 .. 24 */
>> + unsigned bus_number : 8; /* 23 .. 16 */
>> + unsigned device_number : 5; /* 15 .. 11 */
>> + unsigned function_number : 3; /* 10 .. 8 */
>> + unsigned register_number : 6; /* 7 .. 2 */
>> + unsigned zeros : 2; /* 1 .. 0 */
>> +#endif
>> + u32 w;
>> };
>>
>> Pekka
>>
>
> Hehe, this is because it should be rtaher defined as
>
> union pci_config_address {
> struct {
> #if __BYTE_ORDER = __LITTLE_ENDIAN
> unsigned zeros : 2;
> unsigned register_number : 6;
> #else
> ...
> #endif
> }
> u32 w;
> };

Yup, that fixes it for me.

Pekka