|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] tools/hvmloader: implement Intel IGD extended VBT support
On 8/18/2026 3:17 AM, Jan Beulich wrote: > On 17.08.2026 18:04, Chuck Zmudzinski wrote: >> On 8/17/2026 4:42 AM, Jan Beulich wrote: >>> On 14.08.2026 17:23, Chuck Zmudzinski wrote: >>>> On 8/14/2026 9:46 AM, Jan Beulich wrote: >>>>> On 14.08.2026 15:18, Chuck Zmudzinski wrote: >>>>>> On 8/14/2026 3:35 AM, Jan Beulich wrote: >>>>>>> On 14.08.2026 02:45, Chuck Zmudzinski wrote: >>>>>>>> On 8/13/2026 6:35 AM, Jan Beulich wrote: >>>>>>>>> On 02.08.2026 07:08, Chuck Zmudzinski wrote: >>>>>>>>>> -- snip -- >>>>>>>>>> + /* >>>>>>>>>> + * Write rvda_host as 2 successive 32-bit values >>>>>>>>>> + * to communicate location of the VBT to the device >>>>>>>>>> + * model. If rvda_host is not 0, The device model >>>>>>>>>> + * unmaps the OpRegion and eventually maps the VBT >>>>>>>>>> + * after we also write the guest address where the >>>>>>>>>> + * VBT will be mapped. >>>>>>>>>> + * >>>>>>>>>> + * If we send rvda_host = 0 to the device model, it >>>>>>>>>> + * will assume we do not need OpRegion 2 support and >>>>>>>>>> + * it will not unmap the OpRegion. >>>>>>>>>> + */ >>>>>>>>>> + pci_writel(vga_devfn, PCI_INTEL_OPREGION, >>>>>>>>>> + (uint32_t)(rvda_host & 0xfffffffful)); >>>>>>>>>> + unsigned long rvda_host_upper_32 = (uint64_t)rvda_host >> 32; >>>>>>>>>> + pci_writel(vga_devfn, PCI_INTEL_OPREGION, >>>>>>>>>> + (uint32_t)rvda_host_upper_32); >>>>>>>>> >>>>>>>>> Why would you need to communicate a host property to the DM? >>>>>>>> >>>>>>>> The DM cannot access the host rvda value because it is only accessible >>>>>>>> from the host kernel, and the DM is only a user-space process on the >>>>>>>> host. >>>>>>> >>>>>>> I don't follow this: Anything the guest can access should also be >>>>>>> accessible >>>>>>> by its DM. >>>>>> >>>>>> I think the host OpRegion is not currently accessible by the DM. >>>>> >>>>> Can you explain to me how the region becomes accessible to the guest? >>>>> That would then (hopefully) help me understand why the DM would not have >>>>> access. Fundamentally any MMIO and any I/O ports that are assigned to a >>>>> guest are also assigned to its DM. >>>> >>>> Currently, in the device model (Qemu) we have: >>>> >>>> ret = xc_domain_memory_mapping(xen_xc, xen_domid, >>>> (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), >>>> (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), >>>> XEN_PCI_INTEL_OPREGION_PAGES, >>>> DPCI_ADD_MAPPING); >>>> >>>> That statement is in the igd_write_opregion(...) function in the >>>> hw/xen/xen_pt_graphics.c file of the upstream Qemu source. >>>> >>>> If I understand our current implementation correctly, this statement >>>> is what gives the guest access to the host OpRegion (3 pages as defined >>>> by XEN_PCI_INTEL_OPREGION_PAGES, and in agreement with IGD_OPREGION_PAGES >>>> in hvmloader code). >>> >>> No, it introduces mappings of those pages into the guest's P2M. >>> >>>> I don't think this statement makes the host OpRegion >>>> accessible to the device model, though, so I think, if I understand your >>>> comment in an earlier about my patch resulting in what you called a >>>> "layering >>>> violation" correctly, that our current implementation is also guilty of >>>> this >>>> same kind of "layering violation." >>> >>> That code, if it can be successfully executed, indeed doesn't grant any >>> permissions (to the DM or the guest). Instead it proves that the DM has the >>> needed permissions to access the pages itself. >> >> So, are you saying it should be possible, without any patches to either Xen >> or >> the Linux kernel, for Qemu to get a pointer to the OpRegion? If so, how? >> >> I think I could implement what you proposed in an earlier message and do >> all (or most) of this in the DM instead of here in hvmloader: >> >>> The more correct thing to do might be for the DM to >>> put in place a copy before the guest (i.e. hvmloader) even gains control. >>> (How in turn the DM would learn of the contents of the opregion is a >>> separate question then.) >> >> Actually, when I was developing this patch, I tried first to do it that >> way, but the problem was, I could not find a way to get a pointer to the >> host OpRegion in Qemu. >> >> So, how can I get a pointer to the host OpRegion in Qemu? > > You don't ask me this question, do you? Are you offended I asked this question? If so, I am sorry. You make me afraid to ask it again so I will not do so unless you permit to do so again. All I can say is that surely qemu > has an existing way to map (host) physical memory; see e.g. how > xen_pt_msix_init() (imo bogusly) maps the physical MSI-X table of a > device. "Bogusly" there because that's another layering violation. Plus > (independently) there and here there's the issue of how to accomplish > things when not running in Dom0, or when running de-privileged in Dom0. Well, that only proves Qemu *might* be able to access the MSI-X table of a device, that is, if the calls to open /dev/mem and mmap it succeed. Why is the MSI-X table all of the sudden relevant? Even if Qemu can access the MSI-X table of some device, that does not prove that Qemu can access the host OpRegion of an Intel IGD. So I think my point still stands: I still don't see proof that it is possible for Qemu to get a pointer to the host OpRegion without any patches to the current implementations of Xen and the Linux kernel. Perhaps I should accept your indications that it must be possible to get a pointer to the host OpRegion. I will admit maybe it is possible and I have not yet found out how to do it, but I have hardware I can experiment with, and for me, that is the final authority. Proof for me only comes from my own tests and experiments that I conduct on my hardware. Until I see how I can get a pointer to the OpRegion in Qemu on my hardware and actually realize that goal, I remain skeptical that it is possible to do so solely by patching Qemu and not patching either Xen or the Linux kernel. Chuck > > Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |