Fix rescan PCI conflicts with shpchp / pciehp

[Jimmy-Xu]

This is a solution to issue #2460 .

Since the pci rescan will conflict with shpchp / pciehp, we can attach the Nvidia GPU after creating the container, and then the pci rescan will not include the Nvidia GPU.
shpchp / pciehp will handle hot plugging of Nvidia GPU correctly.

There may be better solutions, but this is a working solution.

[bergwolf]

Nice!

[bergwolf]

/test

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@0c3b2c0). Click here to learn what that means.
The diff coverage is 16.66%.

@@            Coverage Diff            @@
##             master    #2461   +/-   ##
=========================================
  Coverage          ?   50.57%           
=========================================
  Files             ?      116           
  Lines             ?    16597           
  Branches          ?        0           
=========================================
  Hits              ?     8394           
  Misses            ?     7177           
  Partials          ?     1026

[devimc]

@Jimmy-Xu does pci-rescan is still needed? I added it in clear-containers because a bug in the pcie-pci-bridges (q35) that was fixed long time ago (qemu > 3.0)

[Jimmy-Xu]

@devimc
I tried to comment out rescanPciBus in kata-agent and this solved my problem. However, I am not sure if it will affect the hot-plugging of other types of devices, so I did not remove it.

So, I added a parameter enable_lazy_attach_device in the kata config, This solution works only when "enable_lazy_attach_device" is true. Currently, only vfio display devices will delay attach.

[devimc]

@Jimmy-Xu I think we can simplify the things a lot with a simple agent's option in the kernel command like, how about adding a new option (agent.no_pci_rescan) in order to don't rescan the PCI bus?

cc @bergwolf

[Jimmy-Xu]

@devimc This is also a solution.

[amshinde]

@Jimmy-Xu I am not in favor of adding a config option in the configuration.toml for delaying attaching devices.
Can we instead have logic in the runtime itself to detect {devices that are hotplugged using pcie native hotplug}/{devices that have a large configuration space} and instead attach those devices in a delayed fashion.
We typically want to have configuration options for supporting diff configurations to the user, I dont think that is the case here.

Also, @devimc pointed out about not rescanning PCI bus again. Here is the background for why the rescan was added in the first place:
kata-containers/agent#380 (comment)

According to that comment and the one following it, we do not need to rescan the PCI bus at all for pc machine type. (@devimc We can probably fix that right away for pc machine). The rescan was added to support attaching devices in case of q35 on a pci-bridge to account for the 5 sec SHPC delay so that we do not have to wait for 5 sec for a device to be detected by the agent. It is certainly not required for devices that are attached using native pcie hotplug.
I dont think we hotplug all devices on pcie ports in case of q35, I think today it is a mix of pci-bridges and pcie ports. So until we get rid of attaching any devices on pci-bridges in case of q35, I dont think we can get rid of the pci bus rescan in case of q35.

[lifupan]

Hi @devimc @amshinde,

In summary, we couldn't remove the rescan for q35 to avoid the 5 sec SHPC delay.
But for those pci devices need much more io spaces shouldn't be rescaned, since the rescan
couldn't reallocate enough IO space for it, and it must depend on the Hotplug mechanism(pcie native hotplug or SHPC). Thus, we should delay those devices hotplug until the "rescan" action had done.

I think @Jimmy-Xu 's way is a good choice: dividing those device into two classes: one was didn't need big io space and could be rescaned, we could hotplug them before the "rescan" action, and the other class is those devices need a big io spaces and depened on the hotplug
mechanism, do the hotplug after the "rescan" action done.

Of course, it's better to do this classification automatically, but I could't figure out an easy way to recognize whether the specific device needs a big IO space or not.

[devimc]

@lifupan

In summary, we couldn't remove the rescan for q35 to avoid the 5 sec SHPC delay.

does kata-agent wait 5 seconds for the device before creating the container? otherwise the workload has to wait 5 seconds until the device is visible in the container

[lifupan]

@lifupan

In summary, we couldn't remove the rescan for q35 to avoid the 5 sec SHPC delay.

does kata-agent wait 5 seconds for the device before creating the container? otherwise the workload has to wait 5 seconds until the device is visible in the container

Hi, @devimc , the answer is no. That's why we shouldn't remove the "rescan" action.

[Jimmy-Xu]

@amshinde

pci rescan is used to ensure that certain devices are connected when the container is created, it will discover all devices connected to the VM via QMP.
shpchp / pciehp is required to hot-plug PCIe devices with large BAR space, with a delay of 5 seconds when the guest operating system starts.

By default, the pci rescan runs before shpchp / pciehp, which prevents proper hot-plugging of PCIe devices.

Since the pci rescan cannot be removed, and the Large BAR space devices require shpchp / pciehp. The solution might be to run pci rescan after shpchp / pciehp woks, or attach the large BAR space devices after creating the container.

For the first solution, the kata-agent needs to check when shpchp / pciehp works, but
Not all solutions require shpchp / pciehp, so kata-agent also needs to distinguish whether shpchp / pciehp is required.

For the second solution, the kata shim can identify large BAR space devices and delay attachment.

In this PR:
I identified all PCIe display VFIO device, then delay to attach them in q35.
enable_lazy_attach_device is just a switch, which is off by default. After all, few devices have Large BAR space.

Another way:
Remove the enable_lazy_attach_device parameter in config. Identify all Large BAR space devices, then delay to attach them by default in q35.

Maybe there is a way to identify Large BAR space device
(run the following command line on the host):

$ lspci -nn  | grep -i nvidia
04:00.0 3D controller [0302]: NVIDIA Corporation Device [10de:15f8] (rev a1)
84:00.0 3D controller [0302]: NVIDIA Corporation Device [10de:15f8] (rev a1)

$lspci -s 04:00.0 -vv | grep Region
	Region 0: Memory at c6000000 (32-bit, non-prefetchable) [disabled] [size=16M]
	Region 1: Memory at 383800000000 (64-bit, prefetchable) [disabled] [size=16G] <<<<<<<
	Region 3: Memory at 383c00000000 (64-bit, prefetchable) [disabled] [size=32M]

$cat /sys/bus/pci/devices/0000\:04\:00.0/resource
0x00000000c6000000 0x00000000c6ffffff 0x0000000000040200
0x0000383800000000 0x0000383bffffffff 0x000000000014220c   <<<<<<<
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000383c00000000 0x0000383c01ffffff 0x000000000014220c
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000000 0x0000000000000000 0x0000000000000000

[Jimmy-Xu]

/test

[Jimmy-Xu]

@amshinde @devimc @bergwolf @lifupan:
I updated the solution:
detect VFIO device, if it is a device with large BAR space, delay to attach it.

[devimc]

thanks @Jimmy-Xu , this pr looks much better 😄

I left some comments

[amshinde]

I updated the solution:
detect VFIO device, if it is a device with large BAR space, delay to attach it.

Thanks @Jimmy-Xu This is what I was looking for, detecting devices that require large BAR space and attaching them after PCI scan. PR looks lot better. I have left a few comments.

[Jimmy-Xu]

/test

[jodh-intel]

Thanks @Jimmy-Xu

[Jimmy-Xu]

/test

[jodh-intel]

Thanks @Jimmy-Xu.

lgtm