On 12.06.2024 16:11, Roger Pau Monné wrote:
> On Wed, Jun 12, 2024 at 03:16:37PM +0200, Jan Beulich wrote:
>> mfn_valid() granularity is (currently) 256Mb. Therefore the start of a
>> 1Gb page passing the test doesn't necessarily mean all parts of such a
>> range would also pass.
>
> How would such a superpage end up in the EPT?
>
> I would assume this can only happen when adding a superpage MMIO that
> has part of it return success from mfn_valid()?
Yes, that's the only way I can think of.
>> Yet using the result of mfn_to_page() on an MFN
>> which doesn't pass mfn_valid() checking is liable to result in a crash
>> (the invocation of mfn_to_page() alone is presumably "just" UB in such a
>> case).
>>
>> Fixes: ca24b2ffdbd9 ("x86/hvm: set 'ipat' in EPT for special pages")
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
Thanks.
>> ---
>> Of course we could leverage mfn_valid() granularity here to do an
>> increment by more than 1 if mfn_valid() returned false. Yet doing so
>> likely would want a suitable helper to be introduced first, rather than
>> open-coding such logic here.
>
> We would still need to call is_special_page() on each 4K chunk,
Why? Within any block for which mfn_valid() returns false, there can be
no RAM pages and hence also no special ones. It's only blocks where
mfn_valid() returns true that we'd need to iterate through page-by-page.
> at
> which point taking advantage of the mfn_valid() granularity is likely
> to make the code more complicated to follow IMO.
Right, this making it more complicated is the main counter argument. Hence
why I think that if to go such a route at all, it would need some new
helper(s) such that at the use sites things still would remain reasonably
clear.
Jan
