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

refactor: add pdev fallback #102

Draft
wants to merge 1 commit into
base: xen-4.14
Choose a base branch
from

Conversation

ejose19
Copy link

@ejose19 ejose19 commented Apr 14, 2021

This PR intends to extend patch-0001-libxl-conditionally-allow-PCI-passthrough-on-PV-with.patch in order to allow using pci_get_pdev when pci_get_pdev_by_domain was unsuccessful, as it seems that update broke something that incorrectly reports the owner of a device thus rendering pci_get_pdev_by_domain useless (see QubesOS/qubes-issues#3217 (comment))

I see the base patch already includes some utilities to make this work, but I would like to ask before proceed:

  • should we import libxl__is_insecure_pv_passthrough_enabled or redefine in this file?
  • should iommu_enabled be stored globally so it's easier to get (as it will probably be used in more patches)
  • is type == LIBXL_DOMAIN_TYPE_PV check redundant in this exact location?

I'm not very knowledgeable in C but given some advise regarding these questions I can complete this PR. Also if you see that this PR should be another flag and not be used implicitly with qubes.enable_insecure_pv_passthrough, please let me know.

@ejose19 ejose19 marked this pull request as draft April 14, 2021 16:24
@marmarek
Copy link
Member

it seems that update broke something that incorrectly reports the owner of a device thus rendering pci_get_pdev_by_domain useless

I'd first like to find one why pci_get_pdev_by_domain fails. Only then it will be possible to find a solution (which may be either fallback to pci_get_pdev, or changes elsewhere to make pci_get_pdev_by_domain return the device).

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.

2 participants