Re: [Xen-devel] [PATCH v3 4/5] x86/boot: Copy 16-bit boot variables back up to Xen image

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.08.2019 18:25, David Woodhouse wrote:
> On Fri, 2019-08-30 at 17:43 +0200, Jan Beulich wrote:
>> On 21.08.2019 18:35, David Woodhouse wrote:
>>> @@ -762,6 +773,11 @@ trampoline_setup:
>>>          mov     $((boot_trampoline_end - boot_trampoline_start) / 4),%ecx
>>>          rep movsl %fs:(%esi),%es:(%edi)
>>>  
>>> +        /* Copy boot data template to low memory. */
>>> +        mov     $sym_offs(bootdata_start),%esi
>>> +        mov     $((bootdata_end - bootdata_start) / 4),%ecx
>>> +        rep movsl %fs:(%esi),%es:(%edi)
>>
>> Afaict neither bootdata_start nor bootdata_end are aligned, and so
>> the difference isn't necessarily a multiple of 4. In fact the
>> other (preexisting) movsl looks to have the same issue; I wonder
>> if we propagate bad EDID data for that reason on certain builds /
>> in certain versions.
> 
> Hm, I'm not sure I quite realised the distinction between
> bootdata_start and __bootdata_start (and likewise _end).
> 
> Now that things are placed in the .data.boot16 section by
> .pushsection/.popsection can we rely on the ordering, and that the
> globals in the .S files are actually at the start and end?

Right now I think we can; as you say further down we may not want to
though.

> I thought we *needed* to use the ones in the linker script, and what I
> should probably do here is kill bootdata_start/bootdata_end completely
> and rely only on the ones from the linker script?
> 
> Either that or I should kill the ones in the linker script completely.

Right, just one pair should survive. And seeing how things work before
this series I think it indeed should be linker script symbols only.
And then the ALIGN() ahead of the "start" ones should stay, but there's
no need for one on the "end" ones (again as is currently the case).

Jan

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