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...
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.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel