2022-08-02 12:10 (UTC-0700), Pallavi Kadam:
> For certain PCIe devices, BAR values are not continuous.
> This patch maps all the BARs and avoids skipping the next BAR addresses.
>
> Fixes: e28aabd88279 ("windows/netuio: introduce NetUIO kernel driver")
> Cc: navas...@microsoft.com
>
> Signed-off-by: Qiao Liu <qiao....@intel.com>
> Signed-off-by: Pallavi Kadam <pallavi.ka...@intel.com>
> ---
> windows/netuio/netuio_dev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/windows/netuio/netuio_dev.c b/windows/netuio/netuio_dev.c
> index b2deb10..e2cac3e 100644
> --- a/windows/netuio/netuio_dev.c
> +++ b/windows/netuio/netuio_dev.c
> @@ -172,6 +172,7 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
> Resources, WDFCMRESLIST R
> ULONG next_descriptor = 0;
> ULONG curr_bar = 0;
> ULONG prev_bar = 0;
> + BOOLEAN bar_done = FALSE;
>
> /*
> * ResourcesTranslated report MMIO BARs in the correct order, but their
> @@ -197,7 +198,8 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
> Resources, WDFCMRESLIST R
> for (INT bar_index = 0; bar_index < PCI_MAX_BAR; bar_index++) {
> prev_bar = curr_bar;
> curr_bar = pci_config.u.type0.BaseAddresses[bar_index];
> - if (curr_bar == 0 || (prev_bar & PCI_TYPE_64BIT)) {
> + if (curr_bar == 0 || ((prev_bar & PCI_TYPE_64BIT) && (bar_done))) {
> + bar_done = FALSE;
> continue;
> }
>
> @@ -219,6 +221,7 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
> Resources, WDFCMRESLIST R
> ctx->bar[bar_index].virt_addr =
> MmMapIoSpace(descriptor->u.Memory.Start,
>
> descriptor->u.Memory.Length,
> MmNonCached);
> + bar_done = TRUE;
> if (ctx->bar[bar_index].virt_addr == NULL) {
> status = STATUS_INSUFFICIENT_RESOURCES;
> goto end;
Hi Pallavi,
In the first place, it was wrong to always test `prev_bar & PCI_TYPE_64BIT`
because only the first BAR slot of a 64-bit BAR contains flags.
The current code has a state to track (curr_bar, prev_bar),
and the fix is complicating it even more without solving the root cause.
I suggest a simpler fix (not tested!)
that eliminates both the incorrectness and the state to maintain:
diff --git a/windows/netuio/netuio_dev.c b/windows/netuio/netuio_dev.c
index d4662b6..8761d31 100644
--- a/windows/netuio/netuio_dev.c
+++ b/windows/netuio/netuio_dev.c
@@ -273,8 +273,6 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
Resources, WDFCMRESLIST R
PCM_PARTIAL_RESOURCE_DESCRIPTOR descriptor;
ULONG next_descriptor = 0;
- ULONG curr_bar = 0;
- ULONG prev_bar = 0;
/*
* ResourcesTranslated report MMIO BARs in the correct order, but their
@@ -298,9 +296,9 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
Resources, WDFCMRESLIST R
* searching for the next MMIO resource each time.
*/
for (INT bar_index = 0; bar_index < PCI_MAX_BAR; bar_index++) {
- prev_bar = curr_bar;
- curr_bar = pci_config.u.type0.BaseAddresses[bar_index];
- if (curr_bar == 0 || (prev_bar & PCI_TYPE_64BIT)) {
+ ULONG bar_value = pci_config.u.type0.BaseAddresses[bar_index];
+
+ if (bar_value == 0) {
continue;
}
@@ -339,6 +337,11 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST
Resources, WDFCMRESLIST R
}
ctx->dpdk_hw[bar_index].mem.size = ctx->bar[bar_index].size;
+
+ // Skip the next BAR slot used by the current 64-bit address.
+ if (bar_value & PCI_TYPE_64BIT) {
+ bar_index++;
+ }
} // for bar_index
end_of_loop:
