Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virttest: Always disable io-reserve on root-port #1462

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ldoktor
Copy link
Contributor

@ldoktor ldoktor commented Mar 27, 2018

it's recommended not to enable io-reserve on pcie-root-port to avoid out
of IO issues when too many devices are plugged there. Some details can
be found here:

https://bugzilla.redhat.com/show_bug.cgi?id=1518278

Signed-off-by: Lukáš Doktor [email protected]

it's recommended not to enable io-reserve on pcie-root-port to avoid out
of IO issues when too many devices are plugged there. Some details can
be found here:

    https://bugzilla.redhat.com/show_bug.cgi?id=1518278

Signed-off-by: Lukáš Doktor <[email protected]>
@@ -1337,6 +1337,10 @@ def sort_key(dev):
for pcic in params.objects("pci_controllers"):
dev = devices.pcic_by_params(pcic, params.object_params(pcic))
pcics.append(dev)
# To avoid SeaBIOS out of IO issues always disable
# "io-reserve" on "pcie-root-port"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does OVMF need same configuration too? @jingzhao84 Could you help confirm? Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vivianQizhu yes, the same configuration with OVMF
@ldoktor did you consider a device actually need to reserve IO when pass a device to the root port ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I did not. How about setting it only when io-reserve is not defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I mean I considered adding it as a configuration, but this seemed more-like default than configuration so I think it should be in code. But I can add a configuration param to be able to enable it. Would that work for you?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ok, thanks

@ldoktor
Copy link
Contributor Author

ldoktor commented Jul 18, 2018

@vivianQizhu should I rebase this, or are other changes required?

@vivianQizhu
Copy link
Contributor

@ldoktor I have talked to Q35 feature owner @jingzhao84 , she thought it is not proper to force all pcie-root-port to io-reserve=0.
I have noticed that in bz 1518278 the devel said "4. We add io-reserve=0 to pcie-root-port to avoid SeaBIOS out of IO issue.", but according to @jingzhao84 that actually still fail the same case in our test.
@jingzhao84 could you help explain more about this? Thanks.

@jingzhao84
Copy link
Contributor

@ldoktor @vivianQizhu the io-reseve=0 is just a hint, the firmware should allocate
IO since there is a device requiring IO connected to the port. Now the parameter mainly affect PCI Devices, such as e1000, when you plug 14 e1000 into pci-root-port, firmware will prompt "PCI: out of I/O address space".
But if I used PCIE devices, such as "virtio-net-pci", "io-reserve=0" didn't affect guest boot up whether we use io-reserve

  1. conventional PCI devices only support to plug into pcie.0 if we used in Q35 machine type
  2. io-reserve only affect PCI devices
    So I don't think we can add "io-reserve=0" as a default parameter.
    Much more tested info and result info, I will list the responding 1518278

Thanks
Jing

@ldoktor
Copy link
Contributor Author

ldoktor commented Jul 19, 2018

Sure, let's move the discussion to: https://bugzilla.redhat.com/show_bug.cgi?id=1518278 and when we reach a conclusion we could update our code...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants