Re: [Xen-devel] [PATCH v7 07/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending (top level ones)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.06.16 at 09:42, <quan.xu@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Jan
>> Beulich
>> Sent: Wednesday, June 08, 2016 10:52 PM
>> To: Xu, Quan <quan.xu@xxxxxxxxx>
>> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; Stefano Stabellini
>> <sstabellini@xxxxxxxxxx>; Wu, Feng <feng.wu@xxxxxxxxx>; Liu Jinsong
>> <jinsong.liu@xxxxxxxxxxxxxxx>; dario.faggioli@xxxxxxxxxx; xen-
>> devel@xxxxxxxxxxxxx; Julien Grall <julien.grall@xxxxxxx>; Suravee
>> Suthikulpanit <suravee.suthikulpanit@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; Keir Fraser <keir@xxxxxxx>
>> Subject: Re: [Xen-devel] [PATCH v7 07/11] IOMMU: propagate IOMMU
>> Device-TLB flush error up to IOMMU suspending (top level ones)
>> 
> 
> On 
>> >>> On 08.06.16 at 10:59, <quan.xu@xxxxxxxxx> wrote:
>> > @@ -169,6 +203,7 @@ static int enter_state(u32 state)
>> 
>> Right above here we have
>> 
>>     if ( (error = device_power_down()) )
>> 
>> which is now wrong as long as SAVED_ALL is not zero.
>> 
>> >      {
>> >          printk(XENLOG_ERR "Some devices failed to power down.");
>> >          system_state = SYS_STATE_resume;
>> > +        device_power_up(error);
>> >          goto done;
>> 
>> For the goto you need to adjust "error", or else you return something
>> meaningless (a sort of random positive number) to your caller.
>> 
> 
> Yes, it is still not correct. Could I change it as following: 
> 
> 
> -    if ( (error = device_power_down()) )
> +    if ( (error = device_power_down()) != SAVED_ALL )
>      {
>          printk(XENLOG_ERR "Some devices failed to power down.");
>          system_state = SYS_STATE_resume;
> +        device_power_up(error);
> +        error = -EIO;
>          goto done;
>      }

This would address only part of the issue afaict - SAVED_ALL
not being zero would still result in the function returning a
positive value instead of zero in the success case. But to be
honest I don't see why this simple to solve an issue requires
any kind of discussion on how to deal with it.

Jan


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