Re: [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

["Laszlo Ersek" <lersek@redhat.com>]

On 07/19/19 12:20, Anthony PERARD wrote:
> On Fri, Jul 05, 2019 at 02:57:06PM +0100, Andrew Cooper wrote:
>> On 04/07/2019 15:42, Anthony PERARD wrote:
>>> diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>>> new file mode 100644
>>> index 0000000000..958195bc5e
>>> --- /dev/null
>>> +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>>> +vtfSignature:
>>> +    DB      'V', 'T', 'F', 0
>>> +
>>> +ALIGN   16
>>> +
>>> +resetVector:
>>> +;
>>> +; Reset Vector
>>> +;
>>> +; This is where the processor will begin execution
>>> +;
>>> +    nop
>>> +    nop
>>
>> Why two nops?
> 
> I don't know, this is existing code that I duplicated to allow adding a
> new entry point. (I wanted to use --find-copies-harder when sending the
> patch, but forgot this time.

Not a big problem; while reviewing v3, I did such comparisons myself, in
my local clone. Feel free to skip "--find-copies-harder" when posting v4
too; I think there isn't much churn going on in parallel right now.

However, a new request for v4: please make sure that the new modules /
paths introduced by this patch set are covered in Maintainers.txt.

> This part of the chunk would not be there.)

Regarding the NOPs: all I can tell you is that they originate from
commit 8332983e2e33 ("UefiCpuPkg: Replace the un-necessary WBINVD
instruction at the reset vector with two NOPs in VTF0.", 2011-08-04).

Whether that change made sense back then, let alone if it makes sense
now: no clue.

Thanks
Laszlo