Yu, Fenghua wrote:
> Avi Kivity wrote:
>>> Han, Weidong wrote:
>>>> In order to support multiple device assignment for KVM, this patch
>>>> does following main changes: - extend dmar_domain to own
>>>> multiple devices from different iommus, use a bitmap of iommus to
>>>> replace iommu pointer in dmar_domain. - add a flag
>>>> DOMAIN_FLAG_VIRTUAL_MACHINE to represent KVM VT-d usage. Many
>>>> functions (e.g. intel_map_single() and intel_unmap_single()) won't
>>>> be used by KVM VT-d. Let them return directly when this flag is
>>>> set.
>>>>
>>>
>>>
>>> This seems brittle. An API that has some functions shorted out
>>> depending on some flag is hard to understand and use.
>
>> This flag just helps identify kvm VT-d usage, and let kvm VT-d APIs
>> reuse >native VT-d code, it needn't duplicate code for kvm VT-d
>> APIs, and it won't >impact existed native VT-d code.
>
>>>
>>> We should either implement the functions, or split the API into a
>>> basic version that talks only to one device, and an expanded
>>> versions that talks to multiple devices, and is implemented by the
>>> using the lower level API. This may require more changes due to
>>> the need to share io pagetables.
>
>> The expanded versions that supports multiple devices will need to
>> change >dmar_domain, this will cause lots of changes, almost
>> duplicate the main >functions.
>
> I would agree with Avi. It's wired to merge the KVM api and native
> api together while they are logically separate. If we have two sets
> of interfaces for KVM and native iommu each, code is clean and robust
> and easy to extend in the future. If you well organize the code, we
> should have both good interface and shared code. While it makes sense
> to share code at beginning of KVM like this, it will get harder to
> maintain and evolve both KVM and native later, e.g. changing one part
> may break the other.
I will remove the flag judgement, and implement independent functions for KVM.
>
> Some comments on the code in this patch:
>
> 1. You don't need to check DOMAIN_FLAG_VIRTUAL_MACHINE in the DMA
> functions. When the DMA functions are called, the domain should not
> have the DOMAIN_FLAG_VIRTUAL_MACHINE flag. Otherwise, the domain
> allocation code is wrong.
Yes, I will remove these meaningless checks.
>
> 2. You need to initialize domain->flags=0 for DMA domain. It's random
> number after the domain is allocated by kmem_cache_alloc.
Right. Thanks.
Regards,
Weidong
>
> Thanks.
>
> -Fenghua
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html