Re: [Xen-devel] [PATCH 2/2] VT-d: section placement and type adjustments

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.10.15 at 08:30, <yang.z.zhang@xxxxxxxxx> wrote:
> Jan Beulich wrote on 2015-09-29:
>> --- a/xen/drivers/passthrough/vtd/intremap.c
>> +++ b/xen/drivers/passthrough/vtd/intremap.c
>> @@ -143,7 +143,7 @@ static void set_hpet_source_id(unsigned
>>      set_ire_sid(ire, SVT_VERIFY_SID_SQ, SQ_13_IGNORE_3,
>> hpetid_to_bdf(id));
>>  }
>> -bool_t iommu_supports_eim(void)
>> +bool_t __init iommu_supports_eim(void)
>>  {
>>      struct acpi_drhd_unit *drhd; unsigned int apic; @@ -832,11 +832,16
>>      @@ int iommu_enable_x2apic_IR(void) struct acpi_drhd_unit *drhd;
>>      struct iommu *iommu;
>> -    if ( !iommu_supports_eim() )
>> -        return -EOPNOTSUPP;
>> +    if ( system_state < SYS_STATE_active )
>> +    {
>> +        if ( !iommu_supports_eim() )
>> +            return -EOPNOTSUPP;
>> 
>> -    if ( !platform_supports_x2apic() )
>> -        return -ENXIO;
>> +        if ( !platform_supports_x2apic() )
>> +            return -ENXIO;
>> +    }
>> +    else if ( !x2apic_enabled )
>> +        return -EOPNOTSUPP;
> 
> Why need the last check here? From the code, this check is called only in 
> resume_x2apic() which already has an assert there: ASSERT(x2apic_enabled) .

Just to cover (theoretical) future callers. Plus I don't think a function
should make undue assumptions about ASSERT()s placed in far away
code, or misbehave in non-debug builds just because then there's no
guard in the caller anymore.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel