Re: [Xen-devel] [PATCH v4 09/10] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On May 13, 2016 2:17 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 13.05.16 at 05:39, <quan.xu@xxxxxxxxx> wrote:
> > On May 10, 2016 5:25 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> >>> On 06.05.16 at 10:54, <quan.xu@xxxxxxxxx> wrote:
> >> > --- a/xen/arch/x86/acpi/power.c
> >> > +++ b/xen/arch/x86/acpi/power.c
> >> >  static int device_power_down(void)  {
> >> > -    console_suspend();
> >> > +    if ( console_suspend() )
> >> > +        return TYPE_CONSOLE;
> >>
> >> This (together with the resume side) makes me guess that the use of
> >> TYPE_ as a prefix confused not just me, but also you:
> >
> > Yes,  this is really not a good prefix, and probably pretty bad to use
> > 'ERROR_'.
> > What about 'PRIOR_'?  then I also need to adjust  device_power_up() as ...
> 
> What about SAVED_?
>

much better, I'll  take it in v5. Thanks.

> >> > -    time_suspend();
> >> > +    if ( time_suspend() )
> >> > +        return TYPE_TIME;
> >> >
> >> > -    i8259A_suspend();
> >> > +    if ( i8259A_suspend() )
> >> > +        return TYPE_I8259A;
> >> >
> >> > +    /* ioapic_suspend should never fail */
> >> >      ioapic_suspend();
> >>
> >> The comment is bogus: "should" means it can in theory. Yet the
> >> function having void return type means it just cannot fail.
> >>
> >
> > I'll use 'cannot', instead of 'should'.
> > Another question, I check the code again, and the rest of the
> > functions (console_suspend/ time_suspend/ i8259A_suspend /
> > ioapic_suspend / lapic_suspend ), in device_power_down(), always returned
> '0'.
> > Maybe I need to fix these functions  annotation from 'int' to 'void',
> > and then I can add a comment on the device_power_down().
> 
> Please don't. Generally the possibility of failure exists, and hence if 
> functions
> have already been written to account for that, we shouldn't strip that
> capability out.
> 

Got it.

Quan

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