On 31/08/2021 14:36, Jan Beulich wrote:
> On 31.08.2021 15:27, Andrew Cooper wrote:
>> On 31/08/2021 14:19, Jan Beulich wrote:
>>>>>> @@ -1095,6 +1101,17 @@ static int __init pvh_setup_acpi(struct
>>>>>> nr_pages = PFN_UP((d->arch.e820[i].addr & ~PAGE_MASK) +
>>>>>> d->arch.e820[i].size);
>>>>>>
>>>>>> + /* Memory below 1MB has been dealt with by pvh_populate_p2m().
>>>>>> */
>>>>>> + if ( pfn < PFN_DOWN(MB(1)) )
>>>>>> + {
>>>>>> + if ( pfn + nr_pages <= PFN_DOWN(MB(1)) )
>>>>>> + continue;
>>>>>> +
>>>>>> + /* This shouldn't happen, but is easy to deal with. */
>>>>> I'm not sure this comment is helpful.
>>>>>
>>>>> Under PVH, there is nothing special about the 1M boundary, and we can
>>>>> reasonably have something else here or crossing the boundary.
>>>> As long as we have this special treatment of the low Mb, the boundary
>>>> is meaningful imo. I'd see the comment go away when the handling in
>>>> general gets streamlined.
>>> I should have added: And as long as Dom0's E820 map gets cloned from
>>> the host's, which will necessarily consider the 1Mb boundary special.
>> Not when you're booting virtualised in the first place.
> You mean when Xen itself runs in PVH (not HVM) mode, and then in turn
> has a PVH Dom0? And if the underlying Xen has not in turn cloned
> the E820 from the host's? That's surely too exotic a case to warrant
> removing this comment. If you insist, I can mention that case as a
> possible exception.
It's a long way from exotic.
Also the magic surrounding the 1M boundary is disappearing on real
hardware with legacy BIOS support being dropped from platforms.
The comment is misleading and should be dropped. The logic is still
perfectly clear given the outer comment.
~Andrew
