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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> 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.

Jan


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