Re: [Xen-devel] [PATCH 4/4] xen/arch: Switch local_irq_restore() to being a static inline helper

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.11.18 at 17:52, <andrew.cooper3@xxxxxxxxxx> wrote:
> RFC, as I expect this patch to get some objection for removing the IRQ safety
> check, but the only reasons that change was made in e5fc6434d7 was because I
> talk talked into doing it while trying to clean up some unnecessary use of
> magic numbers.
> 
> No users are changing any flags (seeing as I've auditing them all in this
> series), and the improvement in emitted code nets us:

Users not changing any flags as per your audit is only one half of it:
I assume you mean the value returned from local_irq_save() gets
passed back unmodified into local_irq_restore(). The other question
is whether intermediately another change to the CPU register has
occurred, which is meant to persist. With Arm also controlling e.g.
enabled status of aborts this way, it may be even more of an issue
there than on x86.


> +static inline void local_irq_restore(unsigned long flags)
> +{
> +    asm volatile ( "push %0; popf;" :: "g" (flags) : "memory" );

I'm afraid this is not entirely right (but perhaps benign): Not
all constants are valid to be handed to PUSH. If you want to
allow immediates in the first place, this would need to be "rme",
I think.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel