Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.02.16 at 07:56, <quan.xu@xxxxxxxxx> wrote:
>> On February 17, 2016 10:23pm, <JBeulich@xxxxxxxx> wrote:
>> >>> On 05.02.16 at 11:18, <quan.xu@xxxxxxxxx> wrote:
>> > to pass down a flag indicating whether the lock is being held, and
>> > check the way up the call trees.
>> 
>> Same comments as on the previous patch; most of the changes outside of
>> xen/drivers/passthrough/ seem to be avoidable here.
>> 
> 
> (VT-d RMRR / P2M EPT)
> 
> Jan,
>    When I fix the VT-d RMRR related code 
>      1. $... iommu_map_page()/iommu_unmap_page() 
> --p2m->set_entry()--p2m_set_entry()--set_identity_p2m_entry() / 
> clear_identity_p2m_entry()-- rmrr_identity_mapping()--... ,
>      2. $... iommu_pte_flush() 
> --p2m->set_entry()--p2m_set_entry()--set_identity_p2m_entry() / 
> clear_identity_p2m_entry()-- rmrr_identity_mapping()--... ,
> 
>  I found that the pcidevs_lock is being held for p2m_set_entry(). It is a 
> corner case which is _not_ fix in previous patch set. As similar, I think I 
> need to add p2m_set_entry_locked() to pass down a flag indicating
>  whether the pcidevs_lock is being held. Right?

Well, to me this would seem rather gross a hack - George, how
about you? I'd really suggest investigating alternatives. One that
comes to mind would be to move acquiring/releasing pcidevs_lock
into a helper function, and setting a per-CPU flag indicating
ownership. However, the same effect could be achieved by making
the lock a recursive one, which would then seem to more
conventional approach (but requiring as much code to be touched).
Both approached would eliminate the need to pass down "locked"
flags.

> BTW, just a quick question, what's the difference between p2m-ept.c and 
> p2m-pt.c ? Thanks.

The former, as its name says, is used for EPT, while the latter is
the P2M implementation for both NPT and shadow mode.

Jan


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