Re: [Xen-devel] [PATCH] IOMMU/x86: avoid pages without GFN in page table creation/updating

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2015-05-11:
>>>> On 11.05.15 at 04:53, <yang.z.zhang@xxxxxxxxx> wrote:
>> Jan Beulich wrote on 2015-05-07:
>>> --- a/xen/drivers/passthrough/x86/iommu.c
>>> +++ b/xen/drivers/passthrough/x86/iommu.c
>>> @@ -59,10 +59,17 @@ int arch_iommu_populate_page_table(struc
>>>          if ( has_hvm_container_domain(d) ||
>>>              (page->u.inuse.type_info & PGT_type_mask) ==
>>> PGT_writable_page )
>>>          {
>>> -            BUG_ON(SHARED_M2P(mfn_to_gmfn(d, page_to_mfn(page)))); - 
>>>           rc = hd->platform_ops->map_page( -                d,
>>> mfn_to_gmfn(d, page_to_mfn(page)), page_to_mfn(page), -               
>>> IOMMUF_readable|IOMMUF_writable); +            unsigned long mfn =
>>> page_to_mfn(page); +            unsigned long gfn = mfn_to_gmfn(d,
>>> mfn); + +            if ( gfn != INVALID_MFN ) +            { +       
>>>         ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH)); +             
>>>   BUG_ON(SHARED_M2P(gfn));
>> 
>> It seems ASSERT() is unnecessary. BUG_ON() is enough to cover it.
> 
> The two check completely different things, so I don't see how the
> BUG_ON() would help with out of bounds, yet also not INVALID_MFN GFNs.
> Please clarify what you mean here.

You are right. I misread the code as BUG_ON(!SHARED_M2P(gfn)).

For Intel VT-d part:

Acked-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>

> 
> Jan


Best regards,
Yang



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