[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

On 3/30/22 1:15 PM, Anthony PERARD wrote:

Hi Chuck,

On Sun, Mar 13, 2022 at 11:41:37PM -0400, Chuck Zmudzinski wrote:

When gfx_passthru is enabled for the Intel IGD, hvmloader maps the IGD
opregion to the guest but libxl does not grant the guest permission to

I'm not reading the same thing when looking at code in hvmloader. It
seems that hvmloader allocate some memory for the IGD opregion rather
than mapping it.


tools/firmware/hvmloader/pci.c:184
     if ( vendor_id == 0x8086 )
     {
         igd_opregion_pgbase = mem_hole_alloc(IGD_OPREGION_PAGES);
         /*
          * Write the the OpRegion offset to give the opregion
          * address to the device model. The device model will trap
          * and map the OpRegion at the give address.
          */
         pci_writel(vga_devfn, PCI_INTEL_OPREGION,
                    igd_opregion_pgbase << PAGE_SHIFT);
     }

I think this write would go through QEMU, does it do something with it?
(I kind of replying to this question at the end of the mail.)

Is this code in hvmloader actually run in your case?


Hi Anthony,

To test your theory that hvmloader is not called in my case and
does nothing, I did the following tests:

I wrote a little C program to read the mapped IGD opregion
address in the guest from the config attribute of the
IGD in sysfs:

#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <stdint.h>

int main()
{
    int fd = open("/sys/devices/pci0000:00/0000:00:02.0/config",
                   O_RDONLY);
    uint32_t buf;
    off_t offset = 0xfc;
    pread(fd, &buf, 4, offset);
    printf("opregion = %lx\n", buf);
    return close(fd);
}
------------------------ end of C program -----------------

Since the config attribute cannot be read by an ordinary user, it is
necessary to run the little C program as the super user to successfully
read the opregion address from sysfs with the little C program.

I also grabbed the BIOS-provided physical RAM map in the
guest which shows the 3 pages mapped by hvmloader for the
opregion (it is the second to last entry in the table):

Mar 31 13:20:16 kolbe kernel: BIOS-provided physical RAM map:
Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x0000000000000000-0x000000000009dfff] usable Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x000000000009e000-0x000000000009ffff] reserved Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x0000000000100000-0x00000000bfbfffff] usable Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000cc346000-0x00000000cc352fff] reserved Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000cf800000-0x00000000df9fffff] reserved Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000fc000000-0x00000000fc009fff] ACPI NVS Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000fc00a000-0x00000000fdffbfff] reserved Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000fdffc000-0x00000000fdffefff] ACPI NVS Mar 31 13:20:16 kolbe kernel: BIOS-e820: [mem 0x00000000fdfff000-0x00000000ffffffff] reserved 

Now with the current code and no patches, the output of the little C
program to print the opregion address from sysfs in the guest is wrong,
the opregion address in the guest is not displayed (it should be
fdffc018 because the offset of the opregion from the page boundary is
0x18 (24) bytes on my hardware) but it displays an error value
(ffffffff) instead:

opregion = ffffffff

I would expect it to be correct if nothing overwrites the value
hvmloader wrote. In fact, I think the value hvmloader wrote might be the
page aligned address of fdffc000 instead of fdffc018. I am not sure
where this error value comes from, so I do need to do some
investigations because I think you are correct that the value that
hvmloader wrote was overwritten somewhere.

Now when I apply my patch to libxl, I get the same physical RAM map
in the guest:

Mar 31 13:12:35 kolbe kernel: BIOS-provided physical RAM map:
Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x0000000000000000-0x000000000009dfff] usable Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x000000000009e000-0x000000000009ffff] reserved Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x0000000000100000-0x00000000bfbfffff] usable Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000cc346000-0x00000000cc352fff] reserved Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000cf800000-0x00000000df9fffff] reserved Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000fc000000-0x00000000fc009fff] ACPI NVS Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000fc00a000-0x00000000fdffbfff] reserved Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000fdffc000-0x00000000fdffefff] ACPI NVS Mar 31 13:12:35 kolbe kernel: BIOS-e820: [mem 0x00000000fdfff000-0x00000000ffffffff] reserved 

And now I get the correct opregion address from the little C program to
read the opregion address from sysfs in the guest, not even the
page-aligned address that hvmloader appears to have written:

opregion = fdffc018

Next I removed the code snippet from hvmloader that allocates three
pages in the guest for the opregion and writes the opregion address to
the pci config attribute, and now there is no memory hole allocated for
the opregion in the guest:

Mar 31 12:47:34 kolbe kernel: BIOS-provided physical RAM map:
Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x0000000000000000-0x000000000009dfff] usable Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x000000000009e000-0x000000000009ffff] reserved Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x0000000000100000-0x00000000bfbfffff] usable Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x00000000cc346000-0x00000000cc352fff] reserved Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x00000000cf800000-0x00000000df9fffff] reserved Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x00000000fc000000-0x00000000fc009fff] ACPI NVS Mar 31 12:47:34 kolbe kernel: BIOS-e820: [mem 0x00000000fc00a000-0x00000000ffffffff] reserved 

I ran this case without my patch to libxl for safety reasons because the
opregion address in sysfs in the guest is wrong and I get the same
error address return value using the little C program to read the
opregion address from sysfs:

opregion = ffffffff

So the conclusion is that hvmloader does allocate the three pages in the
guest for the opregion and writes a value for the opregion address, but
it appears it is overwritten later with the error value when the guest
cannot access the opregion and with the correct value when the guest can
access the opregion.

So I agree that I should understand what is going on here better. I
tentatively think the call to pci_writel in hvmloader can be safely
removed because that value seems to be changed later on somewhere.
But we do need to keep the call to allocate the memory hole for the
opregion in hvmloader, or maybe move that call to the toolstack.

So I will need to have a better answer to your questions before I
propose the next version of the patch / patch series.

Regards,

Chuck

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.