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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2015-10-12:
>>>> 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.

ok, it make sense.

Acked-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>

Best regards,
Yang



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