On 06.12.2022 18:55, Demi Marie Obenour wrote:
> On Tue, Dec 06, 2022 at 12:06:24PM +0000, Andrew Cooper wrote:
>> On 06/12/2022 04:33, Demi Marie Obenour wrote:
>>> @@ -961,13 +1000,24 @@ get_page_from_l1e(
>>>
>>> switch ( l1f & PAGE_CACHE_ATTRS )
>>> {
>>> - case _PAGE_WB:
>>> + default:
>>> +#ifndef NDEBUG
>>> + printk(XENLOG_G_WARNING
>>> + "d%d: Guest tried to use bad cachability attribute %u
>>> for MFN %lx\n",
>>> + l1e_owner->domain_id, l1f & PAGE_CACHE_ATTRS, mfn);
>>
>> %pd. You absolutely want to convert the PTE bits to a PAT value before
>> priniting (decimal on a PTE value is useless), and %PRI_mfn.
>
> I’ll have to look at the rest of the Xen tree to see how to use this.
>
>>> + pv_inject_hw_exception(TRAP_gp_fault, 0);
>>
>> As I said on IRC, we do want this, but I'm not certain if we can get
>> away with just enabling it in debug builds. _PAGE_GNTTAB was ok because
>> it has always been like that, but there's a non-trivial chance that
>> there are existing dom0 kernels which violate this constraint.
>
> I chose this approach because it is very simple to implement. Anything
> more complex ought to be in a separate patch, IMO.
>
>>> + return -EINVAL;
>>> +#endif
As to "complex": Just the "return -EINVAL" would be yet less complex.
Irrespective of the remark my understanding of Andrew's response is that
the concern extends to the error return as well.
Jan
