Re: [Xen-devel] [PATCH v3 16/19] xen/arm: Introduce a macro to synchronize SError

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

On 2017/4/5 16:20, Julien Grall wrote:
> On 05/04/2017 09:08, Wei Chen wrote:
> > Hi Julien,
> >
> > On 2017/4/5 15:29, Julien Grall wrote:
> > > On 05/04/2017 08:14, Wei Chen wrote:
> > > > > Because the ASSERT I suggested was wrong, sorry for that. It should
> > > > > have
> > > > > been:
> > > > >
> > > > > ASSERT(!cpus_have_cap(feat) && local_abort_is_enabled());
> > > > >
> > > > > This is because we want abort enabled when the "feature" is not
> > > > > present.
> > Think more about this assert, I feel confused.
> >
> > Currently, enable abort or not has not combined with the "feature".
> > In my testing, the abort is always enabled in the places where we want
> > to use this macro.
> >
> > Use "SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT" for example,
> >     ASSERT(cpus_have_cap(feat) && local_abort_is_enabled());
> > will panic by when option == DIVERSE, and
> >     ASSERT(!cpus_have_cap(feat) && local_abort_is_enabled());
> > will panic by when option != DIVERSE
> Because I keep making mistake in the ASSERT and I am surprised that
> nobody spot and able to fix...
I think maybe I still in the mood of holiday and my head is not clear
:)

> This should be ASSERT(!cpus_have_cap(feat) || local_abort_is_enabled());
>
> Thinking a bit more, we can also do an ASSERT(local_abort_is_enabled())
> unconditionally.
>
> > I can't see any significance about this change.
> As I said earlier, the main purpose of the ASSERT is to ensure your
> assumption is correct. The abort has been unmasked in the entry but you
> don't know if someone will mask the abort later.
>
> And yes, nobody is playing with the abort mask so far. But are you able
> to predict what will be done in the future? I am not, so the ASSERT is
> here to be sure the abort is unmasked.
I meant change from cpus_have_cap(feat) to !cpus_have_cap(feat) :)

> Cheers,

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