[Xen-devel] [RFC][PATCH][VTD][v2] consolidate VT-d quirks into a single file quirks.c

["Kay, Allen M" <allen.m.kay@xxxxxxxxx>]

>>+static int map_igd_reg(void)
>>+{
>>+    u64 igd_mmio, igd_reg;
>>+    int status;
>>+
>>+    if ( igd_reg_va != NULL )
>>+        return 0;
>>+
>>+    igd_mmio = ((u64)pci_conf_read32(0, INTEL_IGD_DEV, 0, 0x14) << 32) +
>>+                     pci_conf_read32(0, INTEL_IGD_DEV, 0, 0x10);
>>+    igd_reg = (igd_mmio & IGD_BAR_MASK) + 0x2000;
>>+    status = map_pages_to_xen(igd_reg, igd_reg >> PAGE_SHIFT, 1,
>>+                           __PAGE_HYPERVISOR);
>
> You're mapping to *virtual* address igd_reg here? Even if this was
> done only at boot time it would seem wrong, but as I understand it
> this is done so even at run time, which is an absolute no-go.

I don't quite understand this comment.  This mapping is only done once, 
subsequent calls will just return since igd_reg_va will no longer be NULL.  Can 
you elaborate on the reason mapping cannot be done here?  Any suggestion for 
alternatives?

Other than that, I have incorporated you other feedbacks.  See attached patch 
for the changes:

1) backed out new parameter in acpi_parse_dev_scope().  Changed code as you 
suggested.
2) added "volatile" qualifier to mmio read operation in 
cantiga_vtd_ops_preamble()
3) consolidated code for ME phantom function map/unmap.
4) moved initial call to map_igd_reg() to platform_quirks_setup() - not sure if 
this alleviates your concern above...
5) added more comments

Allen

quirks10142.patch
Description: quirks10142.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel