On 22.12.2022 10:50, Demi Marie Obenour wrote:
> On Thu, Dec 22, 2022 at 10:35:08AM +0100, Jan Beulich wrote:
>> On 20.12.2022 02:07, Demi Marie Obenour wrote:
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -6352,6 +6352,11 @@ unsigned long get_upper_mfn_bound(void)
>>> return min(max_mfn, 1UL << (paddr_bits - PAGE_SHIFT)) - 1;
>>> }
>>>
>>> +
>>> +/*
>>> + * A bunch of static assertions to check that the XEN_MSR_PAT is valid
>>> + * and consistent with the _PAGE_* macros, and that _PAGE_WB is zero.
>>> + */
>>> static void __init __maybe_unused build_assertions(void)
>>> {
>>> /*
>>> @@ -6361,6 +6366,72 @@ static void __init __maybe_unused
>>> build_assertions(void)
>>> * using different PATs will not work.
>>> */
>>> BUILD_BUG_ON(XEN_MSR_PAT != 0x050100070406ULL);
>>> +
>>> + /*
>>> + * _PAGE_WB must be zero for several reasons, not least because Linux
>>> + * assumes it.
>>> + */
>>> + BUILD_BUG_ON(_PAGE_WB);
>>
>> Strictly speaking this is a requirement only for PV guests. We may not
>> want to go as far as putting "#ifdef CONFIG_PV" around it, but at least
>> the code comment (and then also the part of the description referring
>> to this) will imo want to say so.
>
> Does Xen itself depend on this?
With the wording in the description I was going from the assumption that
you have audited code and found that we properly use _PAGE_* constants
everywhere.
>>> +} while (0)
>>> +
>>> + /*
>>> + * If one of these trips, the corresponding _PAGE_* macro is
>>> inconsistent
>>> + * with XEN_MSR_PAT. This would cause Xen to use incorrect
>>> cacheability
>>> + * flags, with results that are undefined and probably harmful.
>>
>> Why "undefined"? They may be wrong / broken, but the result is still well-
>> defined afaict.
>
> “undefined” is meant as “one has violated a core assumption that
> higher-level stuff depends on, so things can go arbitrarily wrong,
> including e.g. corrupting memory or data”. Is this accurate? Should I
> drop the dependent clause, or do you have a suggestion for something
> better?
s/undefined/unknown/ ?
Jan
