Re: [Xen-devel] [PATCH v2] x86: correct create_bounce_frame

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/05/17 11:24, Jan Beulich wrote:
>>>> On 05.05.17 at 12:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 05/05/17 10:38, Jan Beulich wrote:
>>>>>> On 05.05.17 at 10:41, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 04/05/17 14:35, Jan Beulich wrote:
>>>>> @@ -345,24 +344,30 @@ UNLIKELY_START(z, create_bounce_frame_ba
>>>>>  __UNLIKELY_END(create_bounce_frame_bad_bounce_ip)
>>>>>          movq  %rax,UREGS_rip+8(%rsp)
>>>>>          ret
>>>>> -        _ASM_EXTABLE(.Lft2,  domain_crash_page_fault_32)
>>>>> -        _ASM_EXTABLE(.Lft3,  domain_crash_page_fault_24)
>>>>> -        _ASM_EXTABLE(.Lft4,  domain_crash_page_fault_8)
>>>>> -        _ASM_EXTABLE(.Lft5,  domain_crash_page_fault_16)
>>>>> -        _ASM_EXTABLE(.Lft6,  domain_crash_page_fault)
>>>>> -        _ASM_EXTABLE(.Lft7,  domain_crash_page_fault)
>>>>> -        _ASM_EXTABLE(.Lft12, domain_crash_page_fault_8)
>>>>> -        _ASM_EXTABLE(.Lft13, domain_crash_page_fault)
>>>>> +        _ASM_EXTABLE(.Lft2,  domain_crash_page_fault_6x8)
>>>>> +        _ASM_EXTABLE(.Lft3,  domain_crash_page_fault_5x8)
>>>>> +        _ASM_EXTABLE(.Lft4,  domain_crash_page_fault_4x8)
>>>>> +        _ASM_EXTABLE(.Lft5,  domain_crash_page_fault_3x8)
>>>> Do you perhaps mean to swap the labels for 4 and 5?
>>> Oh, yes, of course. I remember that something was odd when
>>> doing the conversion, but I wasn't able to spot it because things
>>> looked well ordered.
>>>
>>> Having made a mistake like this I wonder whether it wouldn't be
>>> better to move these next to their instructions anyway? The
>>> way we build them now both insn and extable generation could
>>> actually be bundled into a macro, eliminating any risk for the
>>> two parts to go out of sync ...
>> Do you see such a construct being used anywhere else?  I can't spot any
>> obvious candidates.
> No, just here. But it's eight places.
>
>> For post 4.9, I will be submitting a series which reimplements this
>> logic in C, so I wouldn't expend too much effort in this area.
> True, yet I'm touching all these lines now anyway... Would you
> be opposed to moving them and/or macroizing the whole thing?

Moving the _ASM_EXTABLE() labels is quick and looks like a good thing to do.

Your call on whether macroizing the access is worth the effort or not.

~Andrew

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