In D31150#712854, @whu wrote:

Update the review, incorporate some review comments from Dexuan.

• Rename mana_trc_dbg and likes to mana_dbg, etc.
• Fixed some comments.

Looks good to me. I'll commit it shortly.

In D598#321169, @royger wrote:

Done, MFC'ed as r333106.

I'm hitting a panic with stable/10. Can you please MFC this patch form the HEAD to stable/10?

If there is no new comment, I'm going to commit the patch next week.

I manually tested the patch with releng/{10.3, 10.4, 11.0, 11.1}, stable/{10, 11), and master on local Windows Server 2012 R2, 2016 and Azure, and I don't hit any regression. For Azure testing, I created multiple VMs (Standard D4 v2 (8 vcpus, 28 GB memory) ) in West Central US region and ran a lot of "hot add/remove disks" tests, e.g.
a) hot add and remove a disk on LUN0
b) hot add and remove a disk on LUN1
c) hot add and remove a disk on LUN9
d) hot add and remove a disk on LUN0, and LUN9
e) hot add and remove a disk on LUN1, and LUN9
f) hot add and remove a disk on LUN0~7
g) hot add and remove a disk on LUN0~6, LUN8
h) hot add and remove a disk on LUN1, 9, 10, 11, 12, 13, 14, 15
i) hot add and remove a disk on LUN0, 9, 10, 11, 12, 15, 18, 63
I didn't find any issue.

Sephe will send out a patchset that will handle the issue in a better way.

Sephe will send out a patchset that will handle the issue in a better way.

We need this patch to make PCIe pass-through work for Hyper-V UEFI VM (Generation 2 VM).

Cc'ed more people to have more input.

Fixed the typo: aviable -> available.

In D9686#200243, @marcel wrote:

Going for the workaround I see! :-)

Yes, it's a workaround. Let's try to unblock Hyper-V for the usual case first: currently FreeBSD 10.3, 11, and the HEAD can't boot as UEFI VM on Hyper-V due to this bug.

Thanks! No hurry. This is is only a trivial fix to improve error handling.
We don't have a strong dependency on it at all. Just to make sure I don't forget this patch. :-)

@hselasky, can you please review this patch?

@hselasky, thank you!

@hselasky, thanks Mellanox very much for reviewing and testing the patch!

In D8867#196781, @hselasky wrote:

Hi,

mlx4_core0: Failed to initialize event queue table (err=-12), aborting.

-12 means ENOMEM

I possibly means we're out of memory somehow. Can you track this down.

I assigned enough memory to the VM. :-)

fixed the comment typo pointed out by David.

update the patches per Sephe's comments:

Since sephe's patch (https://reviews.freebsd.org/D8905) is accepted, let me update this patch accordingly.

Let me update the patch, because the previous version causes a sysctl NULL pointer dereferencing issue: we must make sure mlx4_en_sysctl_stat() is called after mlx4_en_create_rx/tx_ring(), otherwise the stat sysctl nodes can't reference the correct ring pointers.

Found a bug in MLX4 VF driver:
in my Hyper-V SR-IOV VF test with MlX ConnectX-3 VF, the VF works fine when the VM has <=12 virtual CPUs, but if the VM has >=13 vCPUs, the VF driver fails to load:

In D8867#185655, @hselasky wrote:

Hi,

The Mellanox FreeBSD team would like to review these changes before they hit the tree. I cannot say exactly when, but it might take a month or two.

--HPS

1. Moved these bits to the end of hn_stop().
2. Call hn_rxfilter_config() by force after the MTU changes.
3. fixed a typo in Summary: "to to" -> "to".

This addressed all the comments.

Moved the new field to the end of the struct.

addressed sephe's comment by using a better method: let's move mlx4_en_sysctl_stat() to an earlier place.

In D8791#182029, @hselasky wrote:

Looks good. Don't forget to MFC to 9/10 and 11. Note that the directory layout is different in 9/10/11 so you'll need to manually extract the patch and apply it. If you think this is difficult I can do the commit for you giving you the credit and handle the MFC-ing.

updated the patch.

addressed jhb's comment

Added a comment before the line "OLD_FILES+=usr/share/man/man4/hv_ata_pci_disengage.4.gz"

hv_ata_pci_disengage.4.gz doesn't exist for ALL architectures any longer.

Don't forget usr/share/man/man4/hv_ata_pci_disengage.4.gz.

updated ObsoleteFiles.inc accordingly (removing the manuals for ARCHs other than amd64 and i386)

Cleaned up some comments.
Ceaned up vmbus_pcib_detach() according to discussions with Hyper-V team.

This new version addressed jhb's comment: "use bus_read_4(hbus->cfg_res, xxx)".

In D8332#175267, @jhb wrote:

This is an interesting approach for pass through. The only other hypervisor I'm familiar with (bhyve) does the config space emulation in the hypervisor itself so that guests don't need modifications for pass through (they just show up as PCI devices).

This new version addressed jhb and sephe's comments.

In D8409#175368, @sepherosa_gmail.com wrote:

In D8409#175275, @jhb wrote:

This looks fine. Note that there is some existing code this somewhat duplicates. For example, the ACPI Host-PCI bridge driver also parses _CRS to determine valid ranges. It uses the pcib_host_res_* API to add those ranges to a list that it then iterates on allocation requests. I'm not sure if does the same trick of preferring > 4G ranges for 64-bit BARs (though if pcib_host_res_*() would otherwise work for you I'd be fine with fixing it to do the same).

Hmm, we probably can reuse the existing APIs, but do two pass decodes, the first pass adds the 64-bits BARs, the second one adds 32-bits BARs.

Good suggestion! I've made a patch with the 2-pass method. Will upload it shortly.

Hi jhb, Thanks for the suggestion! I'll update the patch with the pcib_host_res_* APIs.

I uploaded the new version to address the comments.

This new version addressed Sephe's comments.

This new version addressed Sephe's comment.

The changes to VMBus were moved to 2 separate patches:
https://reviews.freebsd.org/D8410, and
https://reviews.freebsd.org/D8409

removed the get instance guid method, and moved the get_vcpu_id method to a separate patch (https://reviews.freebsd.org/D8410)

In D8071#167026, @hselasky wrote:

Looks good. Do you have access to commit it?

Well, I managed to rebase my patch. Will upload it.

In D8071#167006, @decui_microsoft.com wrote:

In D8071#167001, @hselasky wrote:

Can you also check that the new defines you use, like SYS_RES_IOPORT, are defined for all platforms, and not only i386 and amd64, because the LinuxKPI is building for all platforms.

Thanks for reminding me! I'll check this and rebase my patch later tomorrow -- sorry I can't do it right now, because it's late night here and I'll have to finish some family errands tomorrow first. :-)

In D8071#167005, @hselasky wrote:

Hi,

pci_request_region() was changed to use new function pci_resource_type() which returns the regular SYS_RES_XXX while pci_resource_flags() returns the shifted version.

I think it should be fine?

--HPS

Oh, I overlooked the patch.. sorry.

In D8071#167001, @hselasky wrote:

Can you also check that the new defines you use, like SYS_RES_IOPORT, are defined for all platforms, and not only i386 and amd64, because the LinuxKPI is building for all platforms.

In D8071#167000, @hselasky wrote:

Hi,

Please rebase your patch on top of:
https://svnweb.freebsd.org/changeset/base/306451

Thank you!

--HPS

In D8071#166966, @hselasky wrote:

Hi,

There is a bug that we should return a bitmask from pci_resource_flags() and not a value.

We should return (1<<SYS_RES_IOPORT) etc and the corresponding Linux defines should be updated aswell!

--HPS

Please review v2.
I moved the MEMEN/PORTEN clear/restore from pci_write_bar() to pci_reserve_map().