>>> On 08.02.19 at 18:49, <[email protected]> wrote:
> On Thu, Feb 07, 2019 at 05:21:28PM +0000, George Dunlap wrote:
>> On 2/5/19 1:38 PM, Roger Pau Monné wrote:
>> > On Tue, Feb 05, 2019 at 05:44:14AM -0700, Jan Beulich wrote:
>> >> When the assertion was introduced, MMIO wasn't handled by the
>> >> code correctly anyway (!mfn_valid() MFNs would not have got any
>> >> mappings at all in the 2M and 1G paths), whereas now we have
>> >> p2m_allows_invalid_mfn() there. So the situation has become worse
>> >> with other nearby changes. As a result I think we want to correct
>> >> the assertion here alongside the addition of what you suggest
>> >> above. What about
>> >>
>> >> if ( p2mt != p2m_mmio_direct )
>> >> ASSERT(mfn_valid(mfn) || (mfn_eq(mfn, INVALID_MFN) &&
>> >> p2m_allows_invalid_mfn(p2mt)));
>> >> else
>> >> ASSERT(!mfn_eq(mfn, INVALID_MFN) &&
>> >> !rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn),
>> >> mfn_x(mfn) + PFN_DOWN(MB(2))));
>>
>> FWIW I agree with this approach (asserting !overlaps for p2m_mmio_direct
>> types).
>
> Seeing the report from Sandler:
>
> https://lists.xenproject.org/archives/html/xen-devel/2019-02/msg00578.html
>
> Looks like the assert on the if branch in the above example is not
> correct, the code allows for invalid mfns even if
> p2m_allows_invalid_mfn return false by using l2e_empty.
>
> I think the correct asserts would be:
>
> if ( p2mt == p2m_mmio_direct )
> ASSERT(!mfn_eq(mfn, INVALID_MFN) &&
> !rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn),
> mfn_x(mfn) + PFN_DOWN(MB(2))));
> else
> ASSERT(mfn_valid(mfn) || mfn_eq(mfn, INVALID_MFN));
Hmm, perhaps this is good enough as an assertion, but I'd like
the fixed one to at least be considered:
if ( p2mt == p2m_mmio_direct )
ASSERT(!mfn_eq(mfn, INVALID_MFN) &&
!rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn),
mfn_x(mfn) + PFN_DOWN(MB(2))));
else if ( p2m_allows_invalid_mfn(p2mt) || p2mt == p2m_invalid || p2mt ==
p2m_mmio_dm )
ASSERT(mfn_eq(mfn, INVALID_MFN));
else
ASSERT(mfn_valid(mfn));
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
