From: "Michał Winiarski" <michal.winiarski@intel.com>
To: "Christian König" <christian.koenig@amd.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Bjorn Helgaas" <bhelgaas@google.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
"Michael J . Ruhl" <michael.j.ruhl@intel.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 2/2] PCI: Don't fail BAR resize if nothing is reassigned
Date: Fri, 17 Dec 2021 12:23:40 +0100 [thread overview]
Message-ID: <7a82ddb8-5cb9-aeb8-df20-9f0e2c6fccaa@intel.com> (raw)
In-Reply-To: <60ff4820-5dd9-bb3b-9baa-55ac53c0646c@amd.com>
On 16.12.2021 08:12, Christian König wrote:
> Am 15.12.21 um 15:16 schrieb Michał Winiarski:
>> When pci_reassign_bridge_resources returns -ENOENT, it means that no
>> resources needed to be "moved". This can happen when the resource was
>> resized to be smaller, and it's completely fine - there's no need to
>> treat
>> this as an error and go back to the original BAR size.
>
> Well that doesn't make much sense as far as I can see.
>
> Drivers mandatory need to call pci_release_resource() on all resources
> which might need to move for a resize, including the one which is about
> to be resized.
Since IOV BARs have their own memory-decoding enabled bit, which is
usually tied to the lifetime of virtual functions, the PF driver could
do IOV BAR resize during its lifetime (without releasing its own resources).
>
> When you get -ENOENT from pci_reassign_bridge_resources() it just means
> that the function was not able to do it's work because the driver failed
> to release it's resources before the resize.
>
> Technically we could indeed skip this step if the new size is smaller
> than the old size, but then the question is why would somebody resize in
> the first place? The freed up address space is not usable if you don't
> do this.
With regular BAR, the size of MMIO resource is equal to bar_size.
With IOV BAR, the size of MMIO resource is equal to iov_bar_size *
total_vfs.
It means that the driver could use the pci_resize_resource in two ways,
it could just call it like for the native BAR - overall MMIO resource is
going to grow, or it could limit its total_vfs (overall MMIO resource is
going to shrink, but from VF perspective, its individual BAR is going to
be larger).
To ilustrate:
Native:
1G 2G
+--+ +--+
| | | |
+--+ | |
| |
+--+
Resource grows from 1G to 2G. No surprises.
IOV 4 VFs:
1G 2G
+--+ +--+
| | | |
+--+ | |
| | | |
+--+ +--+
| | | |
+--+ | |
| | | |
+--+ +--+
| |
| |
| |
+--+
| |
| |
| |
+--+
Resource grows from 4G to 8G. But for larger number of VFs, and larger
BAR sizes, finding MMIO space to accomodate may end up being tricky on
some platforms.
IOV (limited to 2 VFs):
1G 2G
+--+ +--+
| | | |
+--+ | |
| | | |
+--+ +--+
| | | |
+--+ | |
| | | |
+--+ +--+
No changes in resource size, we started with 4G and we end up with 4G
after resize (but those 2 VFs can now use 2G BAR).
Does that make sense?
Thanks
-Michał
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
>> ---
>> drivers/pci/setup-res.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index 1946e52e7678a..5de5129055e0a 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -484,7 +484,7 @@ int pci_resize_resource(struct pci_dev *dev, int
>> resno, int size)
>> /* Check if the new config works by trying to assign everything. */
>> if (dev->bus->self) {
>> ret = pci_reassign_bridge_resources(dev->bus->self,
>> res->flags);
>> - if (ret)
>> + if (ret && ret != -ENOENT)
>> goto error_resize;
>> }
>> return 0;
>
next prev parent reply other threads:[~2021-12-17 11:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-15 14:16 [PATCH 0/2] PCI: VF resizable BAR Michał Winiarski
2021-12-15 14:16 ` [PATCH 1/2] PCI: Add support for VF Resizable Bar extended cap Michał Winiarski
2021-12-16 0:21 ` Krzysztof Wilczyński
2021-12-17 12:38 ` Michał Winiarski
2021-12-16 7:50 ` Christian König
2021-12-17 10:58 ` Michał Winiarski
2021-12-15 14:16 ` [PATCH 2/2] PCI: Don't fail BAR resize if nothing is reassigned Michał Winiarski
2021-12-16 7:12 ` Christian König
2021-12-17 11:23 ` Michał Winiarski [this message]
2021-12-17 12:45 ` Christian König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7a82ddb8-5cb9-aeb8-df20-9f0e2c6fccaa@intel.com \
--to=michal.winiarski@intel.com \
--cc=ardb@kernel.org \
--cc=bhelgaas@google.com \
--cc=christian.koenig@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=michael.j.ruhl@intel.com \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox