Julien Grall writes:
> On 27/09/2019 12:56, Volodymyr Babchuk wrote:
>>
>> Julien,
>
> Hi...
>
>>
>> Julien Grall writes:
>>
>>> At the moment, SSBD workaround is re-enabled for Xen after interrupts
>>> are unmasked. This means we may end up to execute some part of the
>>> hypervisor if an interrupt is received before the workaround is
>>> re-enabled.
>>>
>>> As the rest of enter_hypervisor_from_guest() does not require to have
>>> interrupts masked, the function is now split in two parts:
>>> 1) enter_hypervisor_from_guest_noirq() called with interrupts
>>> masked.
>> I'm okay with this approach, but I don't like name for
>> enter_hypervisor_from_guest_noirq(). Right now it is doing exactly one
>> thing - mitigates SSBD. So, maybe more appropriate name will be
>> something like "mitigate_ssbd()" ?
>
> If I wanted to call it mitigate_ssbd() I would have implemented
> completely differently. The reason it is like that is because we may
> need more code to be added here in the future (I have Andrii's series
> in mind). So I would rather avoid a further renaming later on and some
> rework.
Fair enough
>
> Regarding the name, this is a split of
> enter_hypervisor_from_guest(). Hence, why the first path is the
> same. The noirq merely help the user to know what to expect. This is
> better of yet an __ version. Feel free to suggest a better suffix.
I'm bad at naming things :)
I understand that is two halves of one function. But func_name_noirq()
pattern is widely used for other case: when we have func_name_noirq()
function and some func_name() that disables interrupts like this:
void func_name()
{
disable_irqs();
func_name_noirq();
enable_irqs();
}
I like principle of least surprise, so it is better to use some other
naming pattern there.
maybe something like enter_hypervisor_from_guest_pt1() and
enter_hypervisor_from_guest_pt2()?
Or maybe, we should not split the function at all? Instead, we enable
interrupts right in the middle of it.
>
>>
>>> 2) enter_hypervisor_from_guest() called with interrupts unmasked.
>>>
>>> Note that while enter_hypervisor_from_guest_noirq() does not use the
>>> on-stack context registers, it is still passed as parameter to match the
>>> rest of the C functions called from the entry path.
>> As I pointed in the previous email, enter_hypervisor_from_guest() does
>> not use on-stack registers as well.
>
> I am well aware of this, hence my comment here in the commit message
> ;). The reason it is like that is because I wanted to keep the
> prototype the same for all functions called from the entry path (this
> includes do_trap_*).
Let's continue those discussion in the other thread.
[...]
--
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
