RE: [Xen-devel] Problem: Pattern with vertical colored lines on the dom0 screen

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

> The use of magic numbers (0, 2, and 0) here looks like being
> tailored to just a very limited set of systems. If it was really
> known that all current and future systems are going to have this
> arrangement, a comment saying so would be the minimum I'd
> expect.

AFAIK, 0:2.0 has been IGD device for all modern Intel systems with integrated 
graphics.  I can change to use #def.  Other than that, what are the 
alternatives to looking for 0:2.0?  Checking for vendor ID and device CLASS?  
That would preclude any possible discrete graphics from Intel.

> Also, if a check of igd against NULL or a check of type against
> DMAR_TYPE was added here, the two call sites that don't really
> need this output could simply pass NULL instead of adding a new
> local variable.

This I can do.  I'm working on a file that will consolidate all VT-d and IGD 
related quirks.  I will incorporate it.

> Similarly here (a brief comment is there, but to me this doesn't
> mean it's going to be that way forever).
> 
> Also, if this ever needs updating, matching the magic numbers
> here and above is going to be a matter of luck. If they are to be
> hard coded, they should be #define-s, so that locating all uses
> is possible.

Unless you can offer other methods of detecting IGD, I will just change 0:2.0 
to hard coded IGD_BUS, IGD_DEV, IGD_FUNC defines as mentioned above.

>>+}
>>+
>>+static void iommu_enable_translation(struct acpi_drhd_unit *drhd)
>> {
>>     u32 sts;
>>     unsigned long flags;
>>+    struct iommu *iommu = drhd->iommu;
>>+
>>+    if ( !is_igd_vt_enabled() && is_igd_drhd(drhd) )
>>+    {
>>+        if ( force_iommu )
> 
>I would also have suggested switching the checks here: The first
>involves a PCI config space read (and even unconditional upon
>anything), while the second really just is a compare of two
>variables.

I'm not sure I'm convinced you suggestion will make it clearer.  What is the 
rational for making this change?  To me current order makes more sense.  First 
set of changes checks if anything needs to be done. The second check for 
"force_iommu" decides whether to panic or disable IGD VT-d.

Allen


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