Re: [Xen-devel] [PATCH v2] x86/p2m-pt: tighten conditions of IOMMU mapping updates

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.10.15 at 11:47, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Fri, Oct 2, 2015 at 8:31 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>> George Dunlap <george.dunlap@xxxxxxxxxx> 10/01/15 6:16 PM >>>
>>>On 01/10/15 11:25, Jan Beulich wrote:
>>>> TBD: As already mentioned on the large-page-MMIO-mapping patch, there
>>>>      is an apparent inconsistency with PoD handling: 2M mappings get
>>>>      valid entries created, while 4k mappings don't. It would seem to
>>>>      me that the 4k case needs changing, even if today this may only
>>>>      be a latent bug. Question of course is why we don't rely on
>>>>      p2m_type_to_flags() doing its job properly and instead special
>>>>      case certain P2M types.
>>>
>>>The inconsistency in the conditionals there is a bit strange; but I'm
>>>pretty sure that in the 2MB case it is (at the moment) superfluous,
>>>because at the moment it seems that when setting a page with type
>>>p2m_populate_on_demand, it's always passing in _mfn(0), which is valid.
>>>
>>>(It used to pass a magic MFN, but Tim Deegan switched it to _mfn(0) at
>>>some point without comment.)
>>
>> Perhaps just because the magic MFN didn't always work? Tim?
>> To me it looks wrong to pass anything other than INVALID_MFN
>> there.
> 
> Well the magic MFN was also a valid MFN (it was like 1<<9 or
> something), which is probably worse than _mfn(0). :-)
> 
> If we change it to INVALID_MFN, we'd have to either add exceptions in
> all of those conditionals (so it doesn't get l1e_empty()), or perhaps
> (as you say) trust p2m_type_to_flags() to DTRT.  The second is
> probably the best option, but obviously not at this stage in the
> release.

Oh, of course I meant such a change to go into unstable only. Not
the least because surely there is a risk for regressions there.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel