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

Cyrill Gorcunov
2011-12-06T12:10:05Z

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;
};

Cyrill