This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [RFC][PATCH][VTD][v2] consolidate VT-d quirks into a sin

>>> On 15.10.10 at 02:25, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
>> >+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?

The issue is not with doing a mapping, but with where (in virtual
address space) you map to: You're passing a *physical* address
for what is to be a *virtual* one (first argument to
map_pages_to_xen()), i.e. as soon as there is any domain the
mapping will conflict with the domain's use of virtual addresses.
You need to reserve a page somewhere - possibly a fixmap entry
(perhaps the only solution for 32-bit, unless you want to rely on
map_domain_page{,_global}() currently not really depending on
the mapped page being a domain page) -, given that ioremap()
doesn't really exist in Xen.

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

Thanks. There's one more thing I just noticed: Isn't warning you're
adding to is_igd_drhd() going to trigger needlessly (and perhaps
frequently) on Intel IOMMU systems without any device at 0:2.0
I think if at all, the warning should only be issued once.


Xen-devel mailing list