Patch Set 1:

Patch Set 1: Code-Review-1

The error seems valid.

I don't understand how. If we are expecting the resource allocator to not set the IORESOURCE_ASSIGNED flag then it is expected that control would enter pci_set_resource() for a resource that has size 0 and does not have the IORESOURCE_ASSIGNED flag. In that case, the error is not really valid.

The message is not valid. But we must not bail out.


> The size 0 case is explicitly handled to fill
> the bridge registers with sane values. We need a solution that doesn't
> bail out.

I believe you are referring to line 503 in the file? That makes sense to me, but it is really odd that we are saying the the resource does not have the IORESOURCE_ASSIGNED flag, but still has the IORESOURCE_STORED flag(line 499).

So, what are the semantics of these flags:

IORESOURCE_ASSIGNED --> Resource is assigned a value by the allocator
IORESOURCE_STORED --> Either the assignment from allocator is stored in the device or if it is a bridge device without assignment, then assignment to disable the resource is stored in the device.

This would be the result, yes. I would rather like to bend
the meaning of _STORED than that of _ASSIGNED. Currently,
the comment above the former says

    /* The IO resource assignment has been stored in the device */

IMHO, this wasn't true anyway. As there sometimes was no
real assignment.

I'm currently refactoring this function. Will push in a few
minutes.

View Change

To view, visit change 41523. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8116cc8bdcf5a10049330600b1cb060d7880205a
Gerrit-Change-Number: 41523
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Tue, 19 May 2020 22:46:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment