Re: [PATCH v2 for-4.19 1/3] x86/EPT: correct special page checking in epte_get_entry_emt()

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

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