Extra pairs of eyes

[Myles Watson]

I'm tired of staring at this piece of code wondering why printk isn't
working as I expected.  Can someone point out what I've obviously missed?

code (inserted in pci_device.c in pci_get_resource() right before the limit
mask and return):
    if (resource->flags)
    {
    printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx flags
%08lx\n",
                   dev_path(dev), resource->base, resource->limit,
                resource->size, resource->flags);

printk(BIOS_DEBUG, "\t%s size %lx align %lx gran %lx\n",
            dev_path(dev), resource->size,
            resource->align, resource->gran);
    printk(BIOS_DEBUG, " just broken size %08lx\n", resource->size);

printk(BIOS_DEBUG, " broken align %lx\n", resource->align);
    printk(BIOS_DEBUG, "%s resource size %08lx flags %08lx\n",
                   dev_path(dev), resource->size, resource->flags);

printk(BIOS_DEBUG, "%s align %lx gran %lx\n",
            dev_path(dev),
            resource->align, resource->gran);
    }
output:
PCI: 01:00.0 resource base 00000000 limit 00000000 size ffffffff flags
00000000
    PCI: 01:00.0 size 1000 align 0 gran c
 just broken size 00001000
 broken align c
PCI: 01:00.0 resource size 00001000 flags 00000000
PCI: 01:00.0 align c gran c

Notice that size is ffffffff in the first, 0x1000 in the rest.
Align is 0 in the first, c in the rest.

It looks like printk is botching it.  I don't know how else to explain it.
Is there a limit to the number of arguments you can pass to printk?
Thanks,
Myles

[Carl-Daniel Hailfinger]

On 14.10.2008 23:13, Myles Watson wrote:
> I'm tired of staring at this piece of code wondering why printk isn't
> working as I expected.  Can someone point out what I've obviously missed?
>
> code (inserted in pci_device.c in pci_get_resource() right before the limit
> mask and return):
>     if (resource->flags)
>     {
>     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx flags
> %08lx\n",
>                    dev_path(dev), resource->base, resource->limit,
>                 resource->size, resource->flags);
> printk(BIOS_DEBUG, "\t%s size %lx align %lx gran %lx\n",
>         dev_path(dev), resource->size,
>         resource->align, resource->gran);
> printk(BIOS_DEBUG, " just broken size %08lx\n", resource->size);
>
> printk(BIOS_DEBUG, " broken align %lx\n", resource->align);
> printk(BIOS_DEBUG, "%s resource size %08lx flags %08lx\n",
>                dev_path(dev), resource->size, resource->flags);
>
> printk(BIOS_DEBUG, "%s align %lx gran %lx\n",
>         dev_path(dev),
>         resource->align, resource->gran);
> }
>
> output:
> PCI: 01:00.0 resource base 00000000 limit 00000000 size ffffffff flags
> 00000000
>     PCI: 01:00.0 size 1000 align 0 gran c
>  just broken size 00001000
>  broken align c
> PCI: 01:00.0 resource size 00001000 flags 00000000
> PCI: 01:00.0 align c gran c
>
> Notice that size is ffffffff in the first, 0x1000 in the rest.
> Align is 0 in the first, c in the rest.
>
> It looks like printk is botching it.  I don't know how else to explain it.
> Is there a limit to the number of arguments you can pass to printk?
> From a quick glance, this looks strange. Is this v2 or v3? I don't trust

the v3 printk at the moment because I've not reviewed r921 yet.

Oh, and please be aware that any multicore/multiprocessor machine in v3
is completely BROKEN right now and has been that way for months. (Yes, I
have a fix in the queue. Remind me in a few days.)

Regards,
Carl-Daniel

[Myles Watson]

On Tue, Oct 14, 2008 at 3:23 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> On 14.10.2008 23:13, Myles Watson wrote:
> > I'm tired of staring at this piece of code wondering why printk isn't
> > working as I expected.  Can someone point out what I've obviously missed?
> >
> > code (inserted in pci_device.c in pci_get_resource() right before the
>
> limit
> > mask and return):
> >     if (resource->flags)
> >     {
> >     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx
>
> flags
> > %08lx\n",
> >                    dev_path(dev), resource->base, resource->limit,
> >                 resource->size, resource->flags);
> > printk(BIOS_DEBUG, "\t%s size %lx align %lx gran %lx\n",
> >         dev_path(dev), resource->size,
> >         resource->align, resource->gran);
> > printk(BIOS_DEBUG, " just broken size %08lx\n", resource->size);
> >
> > printk(BIOS_DEBUG, " broken align %lx\n", resource->align);
> > printk(BIOS_DEBUG, "%s resource size %08lx flags %08lx\n",
> >                dev_path(dev), resource->size, resource->flags);
> >
> > printk(BIOS_DEBUG, "%s align %lx gran %lx\n",
> >         dev_path(dev),
> >         resource->align, resource->gran);
> > }
> >
> > output:
> > PCI: 01:00.0 resource base 00000000 limit 00000000 size ffffffff flags
> > 00000000
> >     PCI: 01:00.0 size 1000 align 0 gran c
> >  just broken size 00001000
> >  broken align c
> > PCI: 01:00.0 resource size 00001000 flags 00000000
> > PCI: 01:00.0 align c gran c
> >
> > Notice that size is ffffffff in the first, 0x1000 in the rest.
> > Align is 0 in the first, c in the rest.
> >
> > It looks like printk is botching it.  I don't know how else to explain
>
> it.
> > Is there a limit to the number of arguments you can pass to printk?
>
> From a quick glance, this looks strange. Is this v2 or v3?

v3
> I don't trust
> the v3 printk at the moment because I've not reviewed r921 yet.
> Oh, and please be aware that any multicore/multiprocessor machine in v3
> is completely BROKEN right now and has been that way for months. (Yes, I
> have a fix in the queue. Remind me in a few days.)

single core, single processor.

Thanks,
Myles

[ron minnich]

On Tue, Oct 14, 2008 at 2:13 PM, Myles Watson  wrote:
> I'm tired of staring at this piece of code wondering why printk isn't
> working as I expected.  Can someone point out what I've obviously missed?
>
> code (inserted in pci_device.c in pci_get_resource() right before the limit
> mask and return):
>     if (resource->flags)
>     {
>     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx flags
> %08lx\n",
>                    dev_path(dev), resource->base, resource->limit,
>                 resource->size, resource->flags);

typedef u64 resource_t;
struct resource {
        resource_t base;        /* Base address of the resource */
        resource_t size;        /* Size of the resource */
        resource_t limit;       /* Largest valid value base + size -1 */
        unsigned long flags;    /* Descriptions of the kind of resource */
        unsigned long index;    /* Bus specific per device resource id */
        unsigned char align;    /* Required alignment (log 2) of the resource */
        unsigned char gran;     /* Granularity (log 2) of the resource */
        /* Alignment must be >= the granularity of the resource */
};

Look at the type of resource_t. 64 bits.
Your printk is printing 64-bit fields as 32 bits. Things are going to
get very confused.

A common problem.

ron

[Myles Watson]

> -----Original Message-----
> From: ron minnich [mailto:[e-mail]]
> Sent: Tuesday, October 14, 2008 3:29 PM
> To: Myles Watson
> Cc: Coreboot
> Subject: Re: Extra pairs of eyes
>
> On Tue, Oct 14, 2008 at 2:13 PM, Myles Watson  wrote:
> > I'm tired of staring at this piece of code wondering why printk isn't
> > working as I expected.  Can someone point out what I've obviously
>
> missed?
> > code (inserted in pci_device.c in pci_get_resource() right before the
>
> limit
> > mask and return):
> >     if (resource->flags)
> >     {
> >     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx
>
> flags
> > %08lx\n",
> >                    dev_path(dev), resource->base, resource->limit,
> >                 resource->size, resource->flags);
>
> typedef u64 resource_t;
> struct resource {
>         resource_t base;        /* Base address of the resource */
>         resource_t size;        /* Size of the resource */
>         resource_t limit;       /* Largest valid value base + size -1 */
>         unsigned long flags;    /* Descriptions of the kind of resource */
>         unsigned long index;    /* Bus specific per device resource id */
>         unsigned char align;    /* Required alignment (log 2) of the
> resource */
>         unsigned char gran;     /* Granularity (log 2) of the resource */
>         /* Alignment must be >= the granularity of the resource */
> };
>
> Look at the type of resource_t. 64 bits.
> Your printk is printing 64-bit fields as 32 bits. Things are going to
> get very confused.
>
> A common problem.

Should I go through and make all the resource holders resource_t instead of
unsigned long?  They're mixed right now.

I was expecting that my 64-bit values would lose their upper bits, but I
wasn't expecting what I got.

Thanks,
Myles

[ron minnich]

On Tue, Oct 14, 2008 at 7:47 PM, Myles Watson  wrote:
> I was expecting that my 64-bit values would lose their upper bits, but I
> wasn't expecting what I got.

just cast them to u32 in the call to printk

(u32) whatever, (u32) whatever, etc.

ron

[Stefan Reinauer]

Myles Watson wrote:
> I'm tired of staring at this piece of code wondering why printk isn't
> working as I expected.  Can someone point out what I've obviously missed?
>
> code (inserted in pci_device.c in pci_get_resource() right before the
> limit mask and return):
>     if (resource->flags)
>     {
>     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx
> flags %08lx\n",
>                    dev_path(dev), resource->base, resource->limit,
>                 resource->size, resource->flags);
> printk(BIOS_DEBUG, "\t%s size %lx align %lx gran %lx\n",
>         dev_path(dev), resource->size,
>         resource->align, resource->gran);
> printk(BIOS_DEBUG, " just broken size %08lx\n", resource->size);
>
> printk(BIOS_DEBUG, " broken align %lx\n", resource->align);
> printk(BIOS_DEBUG, "%s resource size %08lx flags %08lx\n",
>                dev_path(dev), resource->size, resource->flags);
>
> printk(BIOS_DEBUG, "%s align %lx gran %lx\n",
>         dev_path(dev),
>         resource->align, resource->gran);
> }
>
> output:
> PCI: 01:00.0 resource base 00000000 limit 00000000 size ffffffff flags
> 00000000
>     PCI: 01:00.0 size 1000 align 0 gran c
>  just broken size 00001000
>  broken align c
> PCI: 01:00.0 resource size 00001000 flags 00000000
> PCI: 01:00.0 align c gran c
>
> Notice that size is ffffffff in the first, 0x1000 in the rest.
> Align is 0 in the first, c in the rest.
>
> It looks like printk is botching it.  I don't know how else to explain
> it.  Is there a limit to the number of arguments you can pass to printk?
> Thanks,
> Myles

I have seen this too, with v2, occasionally.

Make sure the printk fetches 4 bytes from the stack when the parameter
is 4 bytes.

Which gcc are you using?

[Myles Watson]

> -----Original Message-----
> From: Stefan Reinauer [mailto:[e-mail]]
> Sent: Tuesday, October 14, 2008 5:18 PM
> To: Myles Watson
> Cc: Coreboot; ron minnich
> Subject: Re: [coreboot] Extra pairs of eyes
>
> Myles Watson wrote:
> > I'm tired of staring at this piece of code wondering why printk isn't
> > working as I expected.  Can someone point out what I've obviously
>
> missed?
> > code (inserted in pci_device.c in pci_get_resource() right before the
> > limit mask and return):
> >     if (resource->flags)
> >     {
> >     printk(BIOS_DEBUG, "%s resource base %08lx limit %08lx size %08lx
> > flags %08lx\n",
> >                    dev_path(dev), resource->base, resource->limit,
> >                 resource->size, resource->flags);
> > printk(BIOS_DEBUG, "\t%s size %lx align %lx gran %lx\n",
> >         dev_path(dev), resource->size,
> >         resource->align, resource->gran);
> > printk(BIOS_DEBUG, " just broken size %08lx\n", resource->size);
> >
> > printk(BIOS_DEBUG, " broken align %lx\n", resource->align);
> > printk(BIOS_DEBUG, "%s resource size %08lx flags %08lx\n",
> >                dev_path(dev), resource->size, resource->flags);
> >
> > printk(BIOS_DEBUG, "%s align %lx gran %lx\n",
> >         dev_path(dev),
> >         resource->align, resource->gran);
> > }
> >
> > output:
> > PCI: 01:00.0 resource base 00000000 limit 00000000 size ffffffff flags
> > 00000000
> >     PCI: 01:00.0 size 1000 align 0 gran c
> >  just broken size 00001000
> >  broken align c
> > PCI: 01:00.0 resource size 00001000 flags 00000000
> > PCI: 01:00.0 align c gran c
> >
> > Notice that size is ffffffff in the first, 0x1000 in the rest.
> > Align is 0 in the first, c in the rest.
> >
> > It looks like printk is botching it.  I don't know how else to explain
> > it.  Is there a limit to the number of arguments you can pass to printk?
> > Thanks,
> > Myles
>
> I have seen this too, with v2, occasionally.
>
> Make sure the printk fetches 4 bytes from the stack when the parameter
> is 4 bytes.
>
> Which gcc are you using?

gcc 4.1.2-13.fc6

Thanks,
Myles